RFR (16): JFR: Separate metadata per JVM-feature

[Roman Kennke]

Hi Markus,

Thank you for your insights. This is fine by me. As I already said in
reply to Erik Gahlin, I thought I'd offer this, but have no strong
opinion either way. I'll withdraw the RFR and Jire issue then.

Cheers,
Roman


> Hi Roman,
> 
> Thanks for suggesting this, I understand what you are trying to
> prove, but I think the cost associated might not be warranted. The
> reason is that when we made JFR open source (JDK 11), we put a lot of
> engineering efforts into consolidation work for the JFR metadata to
> have it be described in a single place, metadata.xml. In the old
> system(s), there used to be many .xml (and .xslt) files scattered
> around, and it was quite painful to understand, manage and process
> them uniformly.
> 
> Unfortunately, this suggestion is a reminder of the old world, which
> have non-favorable connotations, so I would prefer if we can avoid
> doing this.
> 
> Thanks
> Markus
> 
> -----Original Message-----
> From: Roman Kennke <rkennke at redhat.com> 
> Sent: den 23 juli 2020 22:29
> To: Erik Gahlin <erik.gahlin at oracle.com>
> Cc: hotspot-jfr-dev at openjdk.java.net; build-dev <
> build-dev at openjdk.java.net>
> Subject: Re: RFR (16): JFR: Separate metadata per JVM-feature
> 
> Hi Erik,
> 
> 
> > Hi Roman,
> > 
> > What is the problem you are trying to solve? 
> > 
> 
> It is mostly an exercise in cleanliness. In the effort to upstream
> Shenandoah GC to jdk11u (which is still ongoing), it has been asked
> to make sure that build with Shenandoah excluded (--with-jvm-
> features=-
> shenandoahgc) is identical to current build without Shenandoah patch.
> 
> The symbols and empty method(s) compiled in by JFR have been one of a
> few places that needed some work. With this patch and a few more, I
> was able to *prove mechanically* that the objects compiled by
> unpatched JDK11u are byte-identical to patched JDK11u with Shenandoah
> disabled.
> 
> I thought I'd offer this here, maybe it's equally interesting going
> forward. If it's agreed that it is not very interesting then be it -
> I don't have a strong opinion about it.
> 
> Thanks & cheers,
> Roman
> 
> > Having metadata per JVM-features adds complexity with little added 
> > benefit from what I can see.
> > Thanks
> > Erik
> > 
> > > On 23 Jul 2020, at 19:48, Roman Kennke <rkennke at redhat.com>
> > > wrote:
> > > 
> > > This is a fall-out from my Shenandoah upstreaming work in JDK11, 
> > > where I made a similar change in order to separate-out
> > > Shenandoah 
> > > parts from JFR build when Shenandoah is disabled.
> > > 
> > > Currently, all JFR metadata is collected in a single
> > > metadata.xml 
> > > file.
> > > From those, the build machinery generates headers and some other 
> > > things from it. For JVM-feature-specific metadata, this leads to 
> > > stuff generated that doesn't exist when the feature is excluded
> > > from 
> > > the build. For example, when disabling Shenandoah, we still need
> > > to 
> > > compile empty methods in jfrPeriodic.cpp to satisfy the
> > > generated 
> > > code.
> > > 
> > > The fix is to extend the code-generator to accept multiple
> > > metadata-
> > > *.xml files and concatenate the parsing. The version in JDK16 
> > > accepts a --xml $FILENAME argument, and I'm extending this to --
> > > xml 
> > > $FILENAME1:$FILENAME2:.. etc, i.e. multiple filenames separated
> > > by 
> > > colon. It allows empty filenames, e.g. metadata.xml::: would be 
> > > parsed as a single file metadata.xml files. That makes the build 
> > > machinery much simpler.
> > > 
> > > I also did the relevant metadata-separation for Shenandoah
> > > because 
> > > that is what I care about myself. If you'd like other parts
> > > treated 
> > > the same, just let me know.
> > > 
> > > Bug:
> > > https://bugs.openjdk.java.net/browse/JDK-8250218
> > > Webrev:
> > > http://cr.openjdk.java.net/~rkennke/JDK-8250218/webrev.00/
> > > 
> > > Testing: build with and without Shenandoah, run some tests, I'd 
> > > await submit-repo results before pushing.
> > > 
> > > Can I please get a review?
> > > 
> > > Thanks!
> > > Roman
> > >