RFR: JDK-8240224 Allow building hotspot without the serial gc

[Magnus Ihse Bursie]

On 2020-03-09 21:37, Kim Barrett wrote:
>> On Mar 9, 2020, at 4:30 AM, Magnus Ihse Bursie <magnus at ihse.net> wrote:
>>
>> When reworking the JVM feature handling, I wanted to try to compile Hotspot with various features enabled/disabled. I quickly found out that it's not really possible to build hotspot without the serial gc. While this is not a terribly important use case, I think it's good to be able to select serial freely, just as with the other collectors.
>>
>> With this patch it is possible to build a truly minimal JVM using 'configure --with-jvm-variants=custom --with-jvm-features=g1gc'.
>>
>> Bug: https://bugs.openjdk.java.net/browse/JDK-8240224
>> WebRev: http://cr.openjdk.java.net/~ihse/JDK-8240224-building-without-serial-gc/webrev.01
>>
>> /Magnus
> I'm inclined to agree with David and Aleksey that this isn't really a
> worthwhile exercise. Especially not if it involves making some
> otherwise questionable or controversial changes.

As I've said in the previous comments, it's not so much about making 
Hotspot running without Serial GC as making configure live up to it's 
promise not to create an un-buildable configuration. I apologize if my 
changes are questionable or controversial -- my assessment was on the 
contrary that they were simple and non-obtrusive, to the point of 
triviality.

>
> In addition to the issues mentioned by David and Aleksey:
>
> ------------------------------------------------------------------------------
> src/hotspot/share/gc/shared/gcConfig.cpp
>
> I would instead suggest there should not be a default at all instead
> of adding these cases, and the user must explicitly select the GC to
> be used. Since we're talking about an atypical custom build anyway,
> the user presumably knows what they are doing. And yeah, that makes
> the buildjdk stuff elsewhere in this patch harder.

If you build without the Serial GC, it is not even possible to start the 
JVM without a flag selecting GC. Instead you get a somewhat cryptic (and 
incorrect) message about missing garbage collectors. Even if the end 
user would be able to know that you need to pass an additional option 
just to be able to start java, the build system knows no such thing, so 
we cannot even finish the build -- as soon as we try to use the newly 
built JVM (e.g. for running jlink), we will crash and burn.

> Really, I think this ought to just be left alone, along with most of
> the other build-specific changes.
>
> [This also responds to / agrees with Aleksey's comment about this part.]
>
> ------------------------------------------------------------------------------
> src/hotspot/share/gc/shared/genCollectedHeap.cpp
>   197 #if INCLUDE_SERIALGC
>   198   MarkSweep::initialize();
>   199 #endif
>
> This whole file, and several associated files, are *only* used by
> SerialGC now that CMS has been removed: JDK-8234502.

Then maybe they should be excluded when serial is not included? Or, if 
it is determined that Serial GC is essential to hotspot, we should 
remove the INCLUDE_SERIALGC define and associated framework, since it's 
just a fake abstraction if it is not actually possible to build without 
serial GC.

>
> ------------------------------------------------------------------------------
> make/hotspot/lib/JvmFeatures.gmk
>    58 ifeq ($(JVM_VARIANT), custom)
>    59   JVM_CFLAGS_FEATURES += -DVMTYPE=\"Custom\"
>    60 endif
>
> This change looks unrelated to whether serialgc is present or absent.
> If so, it doesn't belong in this changeset at all.

You are correct that this is not strictly about serialgc. When I tested 
my custom build with only epsilongc, I discovered that jtreg barfed on 
the version string produced by the custom JVM build. This is a fix that 
makes sure the VMTYPE always has a value. If you object to me pushing it 
as part of this fix, I can remove it from here and submit it as a 
separate issue. (I just didn't think it was worth the hassle.)

>
> ------------------------------------------------------------------------------
> make/hotspot/lib/JvmFeatures.gmk
> [removed]
>   154   # If serial is disabled, we cannot use serial as OldGC in parallel
>   155   JVM_EXCLUDE_FILES += psMarkSweep.cpp psMarkSweepDecorator.cpp
>
> This was missed by JDK-8235860, which removed those files.  Good find.
... but according to your comment above, that fix also missed to add a 
bunch of other files that should be excluded..? (If we should keep the 
ability to disable serial gc, that is...)
>
> ------------------------------------------------------------------------------
> test/hotspot/gtest/gc/shared/test_collectorPolicy.cpp
>
> As originally written, this test was *only* testing SerialGC. It's not
> obvious that it is actually GC-agnostic and can use the default GC if
> that isn't SerialGC.  Certainly some of the naming suggests otherwise.
> Was this tested with all the other configurations?

No, I have not tested all other configurations. I verified that I could 
build with only g1, only zgc and only epsilongc. I also tested to run 
tier1 testing, and it "mostly" succeeded, but it still failed on several 
tests. My quick eyeballing of the situation indicated that the absolute 
majority (and perhaps all) these failures were related to jtreg tests 
not properly declaring their dependencies on compiler1 or compiler2. 
(Remember, on this bare-bones JVM I only had the interpreter, and 
neither c1 nor c2).

I *could* of course run a suitable set of testing with say c1 and c2 
enabled, and just a single gc enabled, for the set of all gcs != serial 
gc, but then we're *really* getting into the "not worth it" land.

It is not clear to me that the test is only run with Serial GC. As far 
as I can interpret the test framework, this is run with the default 
collector, which typically is *not* serialgc on our testing framework. 
If this is only valid for Serial GC, perhaps the test needs to be amended?

/Magnus

>
> ------------------------------------------------------------------------------
>