RFR: JDK-8239450 Overhaul JVM feature handling in configure

[Erik Joelsson]

Hello Magnus,

This is certainly a nice improvement. It looks good to me. I have some 
comments on implementation details, but nothing serious enough to 
require a new webrev.

Instead of using "echo $foo | sed 's/,/ /g'", since we know we run in 
bash, could just use ${foo//,/ }. (jvm-features.m4:82)

The $AWK expressions are just copied from the previous implementation, 
so they are probably working ok. I would still recommend using $NAWK to 
increase likelihood of them really working the same across platforms.

jvm-features.m4:370,372-373: wrong indentation

/Erik

On 2020-02-19 04:05, Magnus Ihse Bursie wrote:
> The JVM feature handling in configure needs a complete overhaul. The 
> current logic is hard to understand and to make changes to. The method 
> of enabling features with --with-jvm-features="feature list" means 
> that it is not possible to incrementally add features without throwing 
> away settings done earlier on the command line.
>
> With this patch, the most noticeable effect for normal users is the 
> addition of a group of configure arguments, on the pattern 
> --enable-jvm-feature-<FEATURE>. So, instead of doing e.g. 
> --with-jvm-features="zgc,-dtrace", you can now do 
> --enable-jvm-feature-zgc --disable-jvm-feature-dtrace. The major 
> benefit from this is that it is possible to build up a command line in 
> steps, where a later step enables or disables a feature, without 
> throwing away the settings made earlier (which was what happened if 
> two --with-jvm-features= options were given).
>
> Arguably, this is the way that JVM features should have been 
> implemented all along. There were ever only two reasons for the 
> --with-jvm-features argument list, neither of them particularly good: 
> It allows for simple selection of multiple features (e.g. for the 
> custom variant), and it avoided the complexity in programmatically 
> generating autoconf options in m4.
>
> I have now bit the bullet, and wrangled m4 into doing this. The old 
> way of --with-jvm-features="<list>" is of course still supported, but 
> I think for the most part, the new style is recommended. Some 
> features, e.g. cds, had their own options (--enable-cds) which were 
> weirdly translated into features. These options are now defined as 
> aliases (of e.g. --enable-jvm-feature-cds), and I intend to keep them 
> as such.
>
> Under the hood, much more has changed. There is no longer the 
> schizophrenic split between handling stuff the "old" way or the "new" 
> way using features. All features are handled the same, period. 
> Furthermore, the logic has been cleared up considerably.
>
> First of all, I check if a feature is possible to build on your 
> platform. For instance, CDS is not available on AIX, or dtrace 
> requires proper tooling. If that is the case, it is considered 
> unavailable, and it is an error to try to enable it.
>
> This check is also done on a per-variant basis. Some features are not 
> available on all variants. For instance, the "zero" feature is only 
> available on the "zero" variant.
>
> Secondly, not all available features should be enabled by default -- 
> even if most should be. Therefore, I keep lists of "filtered" features 
> (one for the platform and one per variant). The default for a variant 
> is then calculated as all available features, minus all that are 
> filtered out.
>
> If a user has explicitly enabled a feature, it is added to the list 
> (as long as it is available). If a user has disabled a features, it is 
> removed from the list.
>
> This separation makes it clear if a feature is not built by default 
> because it is not supported, or because it is not recommended (like 
> link-time-opt). This distinction was unclear, to say the least, in the 
> old code.
>
> For platforms that are outside my expertise (like zero and aix), I've 
> done my best to convert the old behavior to this model, but I'd 
> appreciate if someone knowledgeable about the code can verify. Zero, 
> for instance, has a long list of features classified as unavailable, 
> but I'm not sure that this is correct.
>
> Hopefully, the new code base should make it much easier to understand 
> what requirements a certain feature has, and why (if not) it is not 
> built. Also, I hope that adding a new feature in the future would be 
> far easier, by just following the model.
>
> Bug: https://bugs.openjdk.java.net/browse/JDK-8239450
> WebRev: 
> http://cr.openjdk.java.net/~ihse/JDK-8239450-overhaul-JVM-features/webrev.01
>
> /Magnus