RFR: 8242039: Improve jlink VersionPropsPlugin

[Langer, Christoph]

Hi Mark,

> -----Original Message-----
> From: mark.reinhold at oracle.com <mark.reinhold at oracle.com>
> Sent: Donnerstag, 2. April 2020 23:13
> To: Langer, Christoph <christoph.langer at sap.com>
> Cc: jigsaw-dev at openjdk.java.net; core-libs-dev at openjdk.java.net
> Subject: Re: RFR: 8242039: Improve jlink VersionPropsPlugin
> 
> 2020/4/2 8:01:28 -0700, christoph.langer at sap.com:
> > please review a small improvement for the jlink
> > VersionPropsPlugin. The Plugin modifies the bytecode of
> > java/lang/VersionProps.class to replace the initializion of certain
> > vendor specific system properties with custom values. This
> > modification currently adds two bytecodes per constant, a pop and
> > another ldc. I overhauled it to simply replace the original ldc of the
> > old value with another ldc, loading the custom value.
> 
> I thought about doing this when I originally wrote that plugin, but it’s
> so awkward to achieve with ASM -- as demonstrated by your patch -- that
> I concluded it wasn’t worth it.  Who will notice an extra pop in a basic
> block that’s only ever executed once?  Is the complexity of this new
> code worth the benefit?

Well, first I started playing with this and got a bit obsessed to find optimizations in that area. (I learned quite a bit about java asm.)
It would be of higher (micro-)benefit for common VM startup if the fields to be modified could be final but that's even more awkward to do and requires intricate knowledge and assumptions about how VersionProps.java is structured. So I decided against messing with that.

Eventually I came up with this result and then I also asked myself the question whether the new complexity was worth the benefit. I answered myself with a yes (though definitely not a clear one ��), and that's why I proposed the change. After all, the new complexity isn't huge...

So, would that be your terminal veto or could you imagine accepting the change?

Best regards
Christoph