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

[Vladimir Kozlov]

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.

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
>>