On 25/07/17 19:28, Roman Kennke wrote:
Can you explain the need to use the 48 bit mask? It is explained here: http://mail.openjdk.java.net/pipermail/shenandoah-dev/2017-June/002756.html
Ah, I see. When needs_explicit_null_check is called from the signal handler this value is coming straight out of info->si_addr (where siginfo_t* info). Hmm, it turns out that this is complicated. I think it's ok for now to push the code as is with one minor change to the comment: - // AArch64 uses 48-bit addresses + // AArch64 addresses passed from the signal handler may have + // their top 8 bits zeroed. That affects the case where + // Shenandoah tries to load a Brooks pointer via a null oop. I have identified why this truncation is happening and what it means. I will start a follow-up thread to discuss why this needs i) a better explanation in the code ii) possible rework "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 :-)
Ok, I guess we can leave this for later generations of devs to go "Tchh, ha!" over.
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.
Hurrah, so it /was/ worth reviewing after all!
Do you think it's ok to push with the size_t changed to jint? Sure. It was arguably 'ok' to push even without my suggested changes but I had to justify my time spent checking it somehow :-)
It was very impressive to see how many improvements you and Aleksey managed to add. Nice job! regards, Andrew Dinn -----------