RFR: JDK-8239450 Overhaul JVM feature handling in configure

Thu Feb 20 16:10:56 UTC 2020

Hello,

On 2020-02-20 01:05, Magnus Ihse Bursie wrote:
> On 2020-02-19 16:00, Erik Joelsson wrote:
>> Hello Magnus,
>>
>> This is certainly a nice improvement. 
> Thanks! It's been long overdue...
>
>> 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)
> Good point. I just copied this from the old code.
>>
>> 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.
> Hm, well, here's the thing. (I had to really check this up.) We set up 
> the following definitions:
> BASIC_REQUIRE_PROGS(NAWK, [nawk gawk awk])
> BASIC_REQUIRE_SPECIAL(AWK, [AC_PROG_AWK])
> and AC_PROG_AWK, according to the documentation, "[c]heck for gawk, 
> mawk, nawk, and awk, in that order".
>
> So, if you have nawk and awk (but no other) installed, both NAWK and 
> AWK will be set to nawk. If you have only awk, both will be set to 
> awk. The difference is if you have gawk installed, then NAWK will be 
> nawk and AWK will be gawk. On my mac, I only have awk, so both AWK and 
> NAWK will be awk. Which works fine with these expressions.
>
> On my ubuntu box, things are even more confused. I have:
> $ ls -l /usr/bin/*awk
> lrwxrwxrwx 1 root root     21 Feb  6 10:36 awk -> /etc/alternatives/awk*
> -rwxr-xr-x 1 root root 658072 Feb 11  2018 gawk*
> -rwxr-xr-x 1 root root   3189 Feb 11  2018 igawk*
> -rwxr-xr-x 1 root root 125416 Apr  3  2018 mawk*
> lrwxrwxrwx 1 root root     22 Feb  6 10:37 nawk -> 
> /etc/alternatives/nawk*
>
> $ ls -l /etc/alternatives/*awk
> lrwxrwxrwx 1 root root 13 Feb 10 10:56 /etc/alternatives/awk -> 
> /usr/bin/gawk*
> lrwxrwxrwx 1 root root 13 Feb 10 10:56 /etc/alternatives/nawk -> 
> /usr/bin/gawk*
>
> So awk, nawk and gawk all executes the same binary, i.e. gawk. Only 
> mawk is different. So on that machine, AWK would be gawk and NAWK 
> would be nawk, but both will execute gawk.
>
> I propose that we remove NAWK, and only use AWK, but we should stop 
> using AC_PROG_AWK and define it in an order that is transparent to us. 
> I recommend [gawk nawk awk], since on Linux systems nawk (as we've 
> seen) is likely to be gawk under disguise anyway, so it's better to be 
> clear about that.
>
> Also, if we truly believe the flavor of awk we're using is having any 
> significance, we should test what version we've actually found, and 
> then either require a specific flavor, and/or test our awk scripts on 
> different awks. But that is better left for another time. For now, 
> I'll keep the AWK. :-)
>
I think the only platform where I've seen it matter is Solaris, but if 
our $AWK is resolved in that order, it doesn't matter. Disregard my comment!

/Erik

>> jvm-features.m4:370,372-373: wrong indentation
> Good catch!
>
> /Magnus
>>
>> /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
>