On 2020-03-09 21:37, Kim Barrett wrote:
On Mar 9, 2020, at 4:30 AM, Magnus Ihse Bursie <[email protected]> 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
------------------------------------------------------------------------------
