[14]RFR: 8227439: Turn off AOT by default

[Igor Ignatyev]

> On Jul 26, 2019, at 6:56 PM, Vladimir Kozlov <vladimir.kozlov at oracle.com> wrote:
> 
> On 7/25/19 12:11 AM, Rahul Raghavan wrote:
>> Hi,
>> Thanks Igor, Vladimir for the review comments.
>> Please review following updates.
>> >> <webrev> - http://cr.openjdk.java.net/~rraghavan/8227439/webrev.01/
>> > You don't need to add -J-XX:+UnlockExperimentalVMOptions to JAOTC_OPTS
>> > in aot/scripts/ scripts because you added it to jaotc launcher
>> > (Launcher-jdk.aot.gmk).
>> >
>> Okay, will removed -J-XX:+UnlockExperimentalVMOptions added to JAOTC_OPTS in -
>> aot/scripts/ - build-bootmodules.sh, build-jdk.vm-modules.sh and test-jaotc.sh
>> > As UseAOT used to be true by default,
>> > you should add to all places where AOTLibrary is used,
>> > e.g. in make/RunTests.gmk,
>> > and as it seems like an easy error (for our users) to make,
>> > I think we need to check that AOTLibrary has value
>> > only if UseAOT is true
>> > and generate an error at initialization time if it's not a case.
>> >
>> Understood the review points by Igor.
>> Found following current implementation of AOTLoader::initialize()
>> http://hg.openjdk.java.net/jdk/jdk/file/9b6d4e64778c/src/hotspot/share/aot/aotLoader.cpp#l110
>> void AOTLoader::initialize() {
>>   ......
>>   if (FLAG_IS_DEFAULT(UseAOT) && AOTLibrary != NULL) {
>>     // Don't need to set UseAOT on command line when AOTLibrary is specified
>>     FLAG_SET_DEFAULT(UseAOT, true);
> 
> Please keep current implementation. No need to change it.
> Examples in AOT JEP use AOTLibrary flag without UseAOT. Lets keep it this way.

Agree, I somehow overlooked this piece in AOTLoader::initialize.

-- Igor
> 
> Vladimir
> 
>>   }
>>   if (UseAOT) {
>>     .......
>>         warning("EagerInitialization is not compatible with AOT (switching AOT off)");
>>     .......
>>         warning("JVMTI capability to post breakpoint is not compatible with AOT (switching AOT off)");
>>     .......
>>         warning("-Xint is not compatible with AOT (switching AOT off)");
>>     // Scan the AOTLibrary option.
>>     if (AOTLibrary != NULL) {
>>       .......
>>     }
>>     // Load well-know AOT libraries from Java installation directory.
>>       .......
>>     }
>>   }
>> }
>> So current design with my webrev.01 is to make UseAOT automatically true if some AOTLibrary is specified and no explicit -UseAOT.
>> Should we change this to -
>> [src/hotspot/share/aot/aotLoader.cpp]
>>    if (FLAG_IS_DEFAULT(UseAOT) && AOTLibrary != NULL) {
>> -    // Don't need to set UseAOT on command line when AOTLibrary is specified
>> -    FLAG_SET_DEFAULT(UseAOT, true);
>> +    if (UseAOT == tue) {
>> +      // Don't need to set UseAOT on command line when AOTLibrary is specified
>> +      FLAG_SET_DEFAULT(UseAOT, true);
>> +    }
>> +    else {
>> +      warning("AOTLibrary specified without explicitly switching on UseAOT (ignoring AOTLibrary)");
>> +    }
>> OR instead of new warning above, throw error -
>> +    else {
>> +      fatal("AOTLibrary specified without explicitly switching on UseAOT");
>> +      vm_exit(1);
>> +    }
>> And then also make sure +UseAOT is added to all places where AOTLibrary is used.
>> Thanks,
>> Rahul
>> On 19/07/19 10:02 PM, Igor Ignatyev wrote:
>>> Hi Rahul,
>>> 
>>> thanks for taking care of this and all the tests, appreciate that. As UseAOT used to be true by default, you should add to all places where AOTLibrary is used, e.g. in make/RunTests.gmk, and as it seems like an easy error (for our users) to make, I think we need to check that AOTLibrary has value only if UseAOT is true and generate an error at initialization time if it's not a case. this will help us and all the users to spot places where AOT was expected to kick in.
>>> 
>>> (I have not looked at all the changed files yet)
>>> 
>>> Thanks,
>>> -- Igor
>>> 
>>>> On Jul 19, 2019, at 12:52 AM, Rahul Raghavan <rahul.v.raghavan at oracle.com> wrote:
>>>> 
>>>> Hi,
>>>> 
>>>> Please review the following fix changeset and
>>>> related release-note task (8228418).
>>>> 
>>>> 
>>>> <webrev> - http://cr.openjdk.java.net/~rraghavan/8227439/webrev.01/
>>>> 
>>>> 
>>>> # https://bugs.openjdk.java.net/browse/JDK-8227439
>>>> (Turn off AOT by default)
>>>> 
>>>> # CSR - https://bugs.openjdk.java.net/browse/JDK-8227833
>>>> 
>>>> # RN - https://bugs.openjdk.java.net/browse/JDK-8228418
>>>> 
>>>> 
>>>> -- AOT support related flags `UseAOT`, `PrintAOT` and `AOTLibrary` are made experimental;
>>>> and `UseAOT` flag is turned off by default.
>>>> Also added required -XX:+UnlockExperimentalVMOptions, related changes in tests.
>>>> 
>>>> -- Got approval for related CSR - 8227833
>>>> and created Release-Note task as commented - 8228418.
>>>> 
>>>> -- tried tests --job hs-tier4,hs-tier4-graal,hs-tier6,hs-tier6-graal.
>>>> Could not find any issues due to proposed changes.
>>>> 
>>>> 
>>>> Please let me know if missed any changes or testing.
>>>> 
>>>> 
>>>> Thanks,
>>>> Rahul