Hi Andrew, thanks for looking through this!
rev 9575 : [backport] implicit null checks broken on aarch64 src/share/vm/asm/assembler.cpp
Can you explain the need to use the 48 bit mask? Is it to do with the subtract of adj making the sign of the result go negative? If so (or even if not :-) then instead of subtracting adj form the offset why not simply add adj to base (suitably scaled) before doing the compare?
I ask because it looks to me like doing this mask operation will mean that we would go on to do an explicit null check when the top 16 bits are non-zero rubbish and the bottom bits happen to be in range. That's not critical because the rubbish bits will get caught at the next check but I just wondered why the mask was needed. It is explained here: http://mail.openjdk.java.net/pipermail/shenandoah-dev/2017-June/002756.html
rev 9597 : Revert G1 changes and bring shared BitMap src/share/vm/gc_implementation/g1/g1CollectedHeap.cpp
In method clean_nmethod(nmethod* nm) the comment says
+ // Mark that this thread has been cleaned/unloaded. + // After this call, it will be safe to ask if this nmethod was unloaded or not. + nm->set_unloading_clock(nmethod::global_unloading_clock());
"thread has been cleaned/unloaded"? Should that not be nmethod? Probably, but this is only a comment, and not even ours. We only re-instanted the code to where it was before (it's G1 code), so this is one minimize-diff-to-upstream change :-) rev ??? [not clear which one it was] src/share/vm/gc_implementation/shenandoah/shenandoahHeapRegion.hpp
+ // Convert to jint with sanity checking + inline static size_t region_size_words_jint() { + assert (ShenandoahHeapRegion::RegionSizeWords <= (size_t)max_jint, "sanity"); + return (jint)ShenandoahHeapRegion::RegionSizeWords; + }
Why is the return type a size_t rather than a jint?
Good catch! This looks like a genuine mistake! I'll push a fix to shenandoah-jdk10 and let it trickle down to jdk9, jdk8 and from there into this. Do you think it's ok to push with the size_t changed to jint? diff --git a/src/share/vm/gc/shenandoah/shenandoahHeapRegion.hpp b/src/share/vm/gc/shenandoah/shenandoahHeapRegion.hpp --- a/src/share/vm/gc/shenandoah/shenandoahHeapRegion.hpp +++ b/src/share/vm/gc/shenandoah/shenandoahHeapRegion.hpp @@ -81,7 +81,7 @@ } // Convert to jint with sanity checking - inline static size_t region_size_words_jint() { + inline static jint region_size_words_jint() { assert (ShenandoahHeapRegion::RegionSizeWords <= (size_t)max_jint, "sanity"); return (jint)ShenandoahHeapRegion::RegionSizeWords; }