RFR (11u, XXL): Upstream/backport Shenandoah to JDK11u - the review thread

[Aleksei Voitylov]

Roman,

so it turned out to be a valuable exercise, indeed! Looking forward for
the script to be able to replicate the check here.

-Aleksei

On 13/07/2020 19:06, Roman Kennke wrote:
> Hi all,
>
> So I did the exercise that we discussed in the other thread, and wrote
> a little script that compares object files of the baseline build with
> the object files of a patched builds with Shenandoah disabled. (I will
> clean up and post the script for this shortly.)
>
> In order for this to make any sense, I needed to patch debug.hpp to
> suppress inclusion of __LINE__ in a bunch of macros. I will clean it up
> and turn this into a proper configure flag and propose it for inclusion
> soon. I think it might be useful more generally (perhaps for JDK16 and
> then trickle down to jdk11u).
>
> This exercise indeed turned up a couple of (harmless) problems where
> Shenandoah code did indeed leak into non-Shenandoah builds:
> - We had a couple of unguarded UseShenandoahGC clauses in C2 code.
> - We generated ShenandoahBarrierNode declarations and node-query in
> node.hpp
> - ADLC build did not pass -DINCLUDE_SHENANDOAHGC=0 when Shenandoah is
> disabled, therefore it would pick up Shenandoah node declarations from
> classes.hpp, which in turn would leak into a couple of places like
> matcher.o, compile.o, dfa.o and a few others.
> - vmOperations.hpp would generate VM_Shenandoah* declarations which
> would get compiled into vmOperations.cpp
> - JFR generated a couple of Shenandoah related events and types, and
> this requires the definition of some empty methods too. I fixed the
> generator so that it can spread the JFR metadata.xml over several
> files, and include GC specific metadata only on-demand. I will extract
> this part and propose it for inclusion upstream in jdk16.
>
> With all those issues fixed, I can (and did) prove that Shenandoah does
> not leak anywhere in libjvm.so when Shenandoah is disabled. It does
> literally compile identical object files. The only exception is the
> inclusion of the global flag UseShenandoah and the code that prints an
> error and exists when trying to run with it. (If really needed, I could
> actually rip it out too, but it doesn't seem worth, nor very clean).
>
> Shenandoah is in-fact cleaner in this respect than any other GC in
> OpenJDK. :-)
>
> There are two places left where Shenandoah has potential impact to
> shared code:
> - Serviceability-agent: we needed to patch some Java code in order to
> provide support for Shenandoah in SA. As far as I can tell, it is dead
> code when running in a Shenandoah-disabled build. (In-fact, it should
> actually work to use the resulting JAR with a build that has Shenandoah
> enabled ;-).
> - Tests: a couple of tests have been amended with Shenandoah
> configurations. All those new configurations are disabled and will not
> be run in a build with Shenandoah disabled. This is also relatively
> easy to prove by actually running the affected tests with Shenandoah-
> disabled builds.
>
> Shared-code changes only:
> http://cr.openjdk.java.net/~rkennke/shenandoah-jdk11u-upstream/webrev.09-shared/
>
> Full webrev:
> http://cr.openjdk.java.net/~rkennke/shenandoah-jdk11u-upstream/webrev.09-all/
>
> What do you think?
>
> Roman
>
>
> On Thu, 2020-07-09 at 19:36 +0200, Roman Kennke wrote:
>> I'm starting a new thread with a new round of webrevs. Maybe we can
>> keep in-principle discussions on the other thread and get some actual
>> reviews here?
>>
>> Anyhow, small changes since the last one:
>> - the lone include "interpreter.hpp" is gone, apparently it was a
>> left-
>> over that's no longer needed
>> - the 2 friend class declarations are now wrapped in
>> INCLUDE_SHENANDOAHGC too, just to be sure (even though compiler
>> wouldn't compile any actual code)
>>
>> As noted in the other thread, there should not be any Shenandoah code
>> leaking out when building with --with-jvm-features=-shenandoahgc
>> (which
>> is the default too). There's 2 exceptions:
>>
>> - The global UseShenandoahGC flag. When invoked but not built-in, it
>> will print an error message and exit.
>> - In:
>> src/hotspot/share/jfr/periodic/jfrPeriodic.cpp
>>
>> I could not exclude the definition of:
>> TRACE_REQUEST_FUNC(ShenandoahHeapRegionInformation)
>>
>> Because that would be called by generated code. Related to this:
>>
>> src/hotspot/share/jfr/metadata/metadata.xml
>>
>> This generates two event classes for Shenandaoh, but they would not
>> be
>> used. I don't know how to exclude them. It seems harmless to me.
>>
>> Shared-only changes:
>> http://cr.openjdk.java.net/~rkennke/shenandoah-jdk11u-upstream/webrev.07-shared/
>>
>> Full webrev:
>> http://cr.openjdk.java.net/~rkennke/shenandoah-jdk11u-upstream/webrev.07-all/
>>
>>
>> Roman
>>