[9] RFR(L) 8166413: Integrate VM part of AOT changes

Thu Nov 3 16:00:15 UTC 2016

On 11/3/16 7:33 AM, Vladimir Kozlov wrote:
> Done:
>
> java -Xlog:aot+class+load=trace -XX:AOTLibrary=./libhelloworld.so 
> HelloWorld
> [0.060s][trace][aot,class,load] found  java.lang.Object  in 
> ./libhelloworld.so for classloader 0x7f1fb80eef30 tid=0x00007f1fb801b000
> ...
>
> I updated webrev with your, Coleen, and Stefan all suggestions:
>
> http://cr.openjdk.java.net/~kvn/aot/hs.webrev.2/
>
> this is delta of changes:
>
> http://cr.openjdk.java.net/~kvn/aot/hs.webrev.delta.2/

Thank you for making these changes.  I must have missed these CompiledIC 
=> const methodHandle& changes when I went through a while ago.

One minor change though:

http://cr.openjdk.java.net/~kvn/aot/hs.webrev.delta.2/src/share/vm/aot/aotCodeHeap.hpp.udiff.html
http://cr.openjdk.java.net/~kvn/aot/hs.webrev.delta.2/src/share/vm/aot/aotLoader.cpp.udiff.html
http://cr.openjdk.java.net/~kvn/aot/hs.webrev.delta.2/src/share/vm/aot/aotLoader.hpp.udiff.html

Since instanceKlassHandle is not a real handle, can you keep that 
passing by value?  I have a script that removes them for jdk 10. Sorry 
for the confusion about these.  The difference is that methodHandle has 
a copy constructor and destructor, so passing by const reference avoids 
copying them.   instanceKlassHandle and KlassHandle are dummy now and 
don't have these so don't add to the code.

I don't need to see another webrev.  Thank you for fixing the logging.

Coleen

>
> Thanks,
> Vladimir
>
> On 11/1/16 3:40 AM, Coleen Phillimore wrote:
>> 5.  Thanks for pointing out the logging tags Stefan. Yes we would 
>> prefer adding "apt" and "fingerprint" and using the composition of 
>> existing tags for logging.
>> Thanks
>> Coleen
>>
>>
>>> On Nov 1, 2016, at 3:38 AM, Stefan Karlsson 
>>> <stefan.karlsson at oracle.com> wrote:
>>>
>>> (resending without formatting)
>>>
>>> Hi Vladimir,
>>>
>>> I just took a quick look at the GC code.
>>>
>>> 1) You need to go over the entire patch and fix all the include 
>>> lines that were added. They are are not sorted, as they should.
>>>
>>> Some examples:
>>>
>>> http://cr.openjdk.java.net/~kvn/aot/hs.webrev/src/share/vm/memory/metaspace.cpp.udiff.html 
>>>
>>>
>>>  #include "utilities/debug.hpp"
>>>  #include "utilities/macros.hpp"
>>> + #include "aot/aotLoader.hpp"
>>>
>>>
>>> http://cr.openjdk.java.net/~kvn/aot/hs.webrev/src/share/vm/gc/g1/g1RootProcessor.cpp.udiff.html 
>>>
>>>
>>>  #include "gc/g1/g1Policy.hpp"
>>>  #include "gc/g1/g1RootClosures.hpp"
>>>  #include "gc/g1/g1RootProcessor.hpp"
>>>  #include "gc/g1/heapRegion.inline.hpp"
>>>  #include "memory/allocation.inline.hpp"
>>> + #include "aot/aotLoader.hpp"
>>>  #include "runtime/fprofiler.hpp"
>>>  #include "runtime/mutex.hpp"
>>>  #include "services/management.hpp"
>>>
>>> 2) I'd prefer if the the check if AOT is enabled was folded into 
>>> AOTLoader::oops_do, so that the additions to the GC code would be 
>>> less conspicuous.
>>>
>>> For example: 
>>> http://cr.openjdk.java.net/~kvn/aot/hs.webrev/src/share/vm/gc/parallel/psMarkSweep.cpp.udiff.html
>>>
>>>    CodeBlobToOopClosure adjust_from_blobs(adjust_pointer_closure(), 
>>> CodeBlobToOopClosure::FixRelocations);
>>>    CodeCache::blobs_do(&adjust_from_blobs);
>>> +   if (UseAOT) {
>>> +     AOTLoader::oops_do(adjust_pointer_closure());
>>> +   }
>>>    StringTable::oops_do(adjust_pointer_closure());
>>>    ref_processor()->weak_oops_do(adjust_pointer_closure());
>>> PSScavenge::reference_processor()->weak_oops_do(adjust_pointer_closure()); 
>>>
>>>
>>> Would be:
>>>
>>>    CodeBlobToOopClosure adjust_from_blobs(adjust_pointer_closure(), 
>>> CodeBlobToOopClosure::FixRelocations);
>>>    CodeCache::blobs_do(&adjust_from_blobs);
>>> +   AOTLoader::oops_do(adjust_pointer_closure());
>>>    StringTable::oops_do(adjust_pointer_closure());
>>>    ref_processor()->weak_oops_do(adjust_pointer_closure());
>>> PSScavenge::reference_processor()->weak_oops_do(adjust_pointer_closure()); 
>>>
>>>
>>> 3) aotLoader.hpp uses implements methods using GrowableArray. This 
>>> will expose the growable array functions to all includers of that 
>>> file. Please move all that code out to an aotLoader.inline.hpp file, 
>>> and then remove the unneeded includes from the aotLoader.hpp file. 
>>> 4) 
>>> http://cr.openjdk.java.net/~kvn/aot/hs.webrev/src/share/vm/memory/virtualspace.hpp.udiff.html
>>>
>>>    // Reserved area
>>>    char* low_boundary()  const { return _low_boundary; }
>>>    char* high_boundary() const { return _high_boundary; }
>>>
>>> +   void set_low_boundary(char *p)  { _low_boundary = p; }
>>> +   void set_high_boundary(char *p) { _high_boundary = p; }
>>> +   void set_low(char *p)           { _low = p; }
>>> +   void set_high(char *p)          { _high = p; }
>>> +
>>>    bool special() const { return _special; }
>>>
>>> These seems unsafe to me, but that might be because I don't 
>>> understand how this is used. VirtualSpace has three sections, the 
>>> lower, middle, and the high. The middle section might have another 
>>> alignment (large pages) than the others. Is this property still 
>>> maintained when these functions are used?
>>>
>>>
>>> 5) 
>>> http://cr.openjdk.java.net/~kvn/aot/hs.webrev/src/share/vm/logging/logTag.hpp.udiff.html
>>>
>>> Did you discuss with the Runtime team about the naming of these 
>>> tags? The other class* tags where split up into multiple tags. For 
>>> example, classload was changed to class,load.
>>>
>>> Thanks,
>>> StefanK
>>>
>>>> On 27/10/16 03:15, Vladimir Kozlov wrote:
>>>> AOT JEP:
>>>> https://bugs.openjdk.java.net/browse/JDK-8166089
>>>> Subtask:
>>>> https://bugs.openjdk.java.net/browse/JDK-8166415
>>>> Webrev:
>>>> http://cr.openjdk.java.net/~kvn/aot/hs.webrev/
>>>>
>>>> Please, review Hotspot VM part of AOT changes.
>>>>
>>>> Only Linux/x64 platform is supported. 'jaotc' and AOT part of 
>>>> Hotspot will be build only on Linux/x64.
>>>>
>>>> AOT code is NOT linked during AOT libraries load as it happens with 
>>>> normal .so libraries. AOT code entry points are not exposed (not 
>>>> global) in AOT libraries. Only class data has global labels which 
>>>> we look for with dlsym(klass_name).
>>>>
>>>> AOT-compiled code in AOT libraries is treated by JVM as *extension* 
>>>> of existing CodeCache. When a java class is loaded JVM looks if 
>>>> corresponding AOT-compiled methods exist in loaded AOT libraries 
>>>> and add links to them from java methods descriptors (we have new 
>>>> field Method::_aot_code). AOT-compiled code follows the same 
>>>> invocation/deoptimization/unloading rules as normal JIT-compiled code.
>>>>
>>>> Calls in AOT code use the same methods resolution runtime code as 
>>>> calls in JITed code. The difference is call's destination address 
>>>> is loaded indirectly because we can't patch AOT code - it is 
>>>> immutable (to share between multiple JVM instances).
>>>>
>>>> Classes and Strings referenced in AOT code are resolved lazily by 
>>>> calling into runtime. All mutable pointers (oops (mostly strings), 
>>>> metadata) are stored and modified in a separate mutable memory (GOT 
>>>> cells) - they are not embedded into AOT code.
>>>>
>>>> Changes includes klass fingerprint generation since we need it to 
>>>> find correct klass data in loaded AOT libraries.
>>>>
>>>> Thanks,
>>>> Vladimir
>>>
>>>
>>