Matias cds + logging/RedefineClasses.java crash fix. #1775
Conversation
|
👋 Welcome back coleenp! A progress list of the required criteria for merging this PR into |
|
❗ This change is not yet ready to be integrated. |
|
@coleenp this pull request can not be integrated into git checkout matias-cds
git fetch https://git.openjdk.org/valhalla.git lworld
git merge FETCH_HEAD
# resolve conflicts and follow the instructions given by git merge
git commit -m "Merge lworld"
git push |
openjdk
bot
added
the
merge-conflict
| void print_elements_on(outputStream* st) { | ||
| FlatArrayKlass::cast(real_klass())->oop_print_elements_on(raw_flatArrayOop(), st); | ||
| } | ||
|
|
There was a problem hiding this comment.
AOTMapLogger::FakeRefArray has an FakeOop obj_at(int i) method for reading the elements. Do we need an equivalent for FakeFlatArray?
There was a problem hiding this comment.
Maybe we get away without having it due to the implementation of FlatArrayKlass::oop_print_on?
There was a problem hiding this comment.
I thought it was nicer to have the printing logic in the flatArrayOop.cpp file rather than in some AOT file, so it's more like typeArrayOop. The refArray case wants to save the oops it finds for something.
There was a problem hiding this comment.
Although I suppose this won't work if the flatArrayKlass has oops (something about read_oop_at())
There was a problem hiding this comment.
This code is weird. I don't know this code.
| // Save a C heap allocated version of the scalarized signature and store it in the adapter | ||
| GrowableArray<SigEntry>* heap_sig = new (mtInternal) GrowableArray<SigEntry>(ces.sig_cc()->length(), mtInternal); | ||
| heap_sig->appendAll(ces.sig_cc()); | ||
| super_method->adapter()->set_sig_cc(heap_sig); |
There was a problem hiding this comment.
I don't understand (yet) why the scalarized versions aren't already archived as part of the existing adapter archiving. Getting them saved/restored in the archive seems like the best approach, with this being a work around until that's sorted
There was a problem hiding this comment.
We explicitly null out the _sig_cc from the archived adapters in AdapterHandlerEntry::remove_unshareable_info as _sig_cc is a new Valhalla thing. It's possible for it to be added to the AOTCache, but we just haven't done the work to save / restore the arrays of SigEntries.
Opened https://bugs.openjdk.org/browse/JDK-8373348 a reminder to update the AOTCache support later
|
|
||
| bool CDSConfig::is_dumping_heap() { | ||
| if (is_valhalla_preview()) { | ||
| // Not working yet -- e.g., HeapShared::oop_hash() needs to be implemented for value oops |
There was a problem hiding this comment.
I don't see an implementation of HeapShared::oop_hash() for value oops in this PR. Can you point me at it or is this comment outdated?
There was a problem hiding this comment.
This is something that doesn't work that for some reason I didn't run into on Friday.
There was a problem hiding this comment.
It wasn't the oop_hash() reason that something didn't work, it was the wrong prototype header.
openjdk
bot
removed
the
merge-conflict
…w to javac if java is run with enable preview.
openjdk
bot
added
the
merge-conflict
2 participants
Progress
Error
Warnings
Reviewing
Using
gitCheckout this PR locally:
$ git fetch https://git.openjdk.org/valhalla.git pull/1775/head:pull/1775$ git checkout pull/1775Update a local copy of the PR:
$ git checkout pull/1775$ git pull https://git.openjdk.org/valhalla.git pull/1775/headUsing Skara CLI tools
Checkout this PR locally:
$ git pr checkout 1775View PR using the GUI difftool:
$ git pr show -t 1775Using diff file
Download this PR as a diff file:
https://git.openjdk.org/valhalla/pull/1775.diff