Hi Magnus,

Thanks for your review and valuable comments.
It is the second case since the VM will report an error for DumpLoadedClassList when CDS is disabled.
So generate-classlist *never* will work with CDS disabled.

Updated: http://cr.openjdk.java.net/~jiefu/8232768/webrev.01/

Please review it and give me some advice.

Thanks a lot.
Best regards,
Jie

On 2019/10/22 下午5:54, Magnus Ihse Bursie wrote:
On 2019-10-22 09:44, Jie Fu wrote:
Hi all,

May I get reviews for this one-line change?

JBS:    https://bugs.openjdk.java.net/browse/JDK-8232768
Webrev: http://cr.openjdk.java.net/~jiefu/8232768/webrev.00/

This kind of configuration, with both --disable-cds and --enable-generate-classlist, will fail. The failure is caused by -XX:DumpLoadedClassList [1], which doesn't work as expected when cds is disabled.

It might be better to produce an error.
I'm not sure I agree with this fix. Checking the code, we make an effort to determine if we should warn the user:

  # Check if it's likely that it's possible to generate the classlist. Depending
  # on exact jvm configuration it could be possible anyway.
  if test "x$ENABLE_CDS" = "xtrue" && (HOTSPOT_CHECK_JVM_VARIANT(server) || HOTSPOT_CHECK_JVM_VARIANT(client) || HOTSPOT_CHECK_JVM_FEATURE(cds)); then

Only if the user *explicitly* sets --enable-generate-classlist, the warning is printed. The comment clearly states that this might be possible anyway, given the circumstances, so I believe a warning, and not an error, is the right thing here. The user explicitly requests generate-classlist. We warn that it might not work, but we allow it since it can work in cases we don't really know about.

If it is the case that generate-classlist *never* will work with CDS disabled (this sounds reasonable, but I don't know if this really is the case), and you want to catch this, then I suggest you restructure the code so you catch the CDS && generate-classlist case separately, and give a fatal error for that, and then keep the current check for jvm variants/features and let it keep producing a warning for that.

/Magnus

Thanks a lot.
Best regards,
Jie

[1] http://hg.openjdk.java.net/jdk/jdk/file/ca620b06b5c9/make/GenerateLinkOptData.gmk#l68




Reply via email to