[aarch64-port-dev ] RFR: Backport Shenandoah since last merge
Roman Kennke
rkennke at redhat.com
Tue Jul 25 18:28:42 UTC 2017
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;
}
More information about the aarch64-port-dev
mailing list