RFR (XL): 8152664 - Support non-continuous CodeBlobs in HotSpot
Rickard Bäckman
rickard.backman at oracle.com
Tue Apr 19 05:20:27 UTC 2016
Volker,
I'm fine with 8151956 going in as a separate commit. I can sponsor that
change and will push it at the same time as I push this (8152664) change.
Please put it out for review as soon as possible.
Thanks
/R
On 04/18, Volker Simonis wrote:
> Hi Rickard,
>
> are you fine if we are fixing the issue about non-initialized nmethods
> independently under "8151956 : CodeBlob ctor virtual call on partially
> constructed subclass"? In that case I'll submit an extra RFR today
> with that fix only. Anyway you or Dean will have to sponsor it because
> it is in shared code.
>
> Or do you want to fix the issue together with your change for 8152664?
>
> Anyway, after we've fixed the problem with the partially constructed
> classes I think we should make is_compiled() and is_nmethod() virtual
> again and get rid of the "subtype()" hack:
>
> http://cr.openjdk.java.net/~simonis/webrevs/2016/8152664_addon/
>
> (This is relatively to your webrev.)
>
> Regards,
> Volker
>
>
> On Fri, Apr 15, 2016 at 11:19 PM, Dean Long <dean.long at oracle.com> wrote:
> >
> > On 4/15/2016 11:28 AM, Volker Simonis wrote:
> >>
> >> Hi,
> >>
> >> this one was a real puzzler :)
> >> But I finally found some quiet hours in the office today and came up
> >> with this rather simple solution:
> >>
> >> http://cr.openjdk.java.net/~simonis/webrevs/2016/8151956/
> >>
> >> First I've tried to only move the relocation handling out of the
> >> CodeBlob constructor into the derived constructors but that didn't
> >> work very well and was quite complicated. So finally, I've just moved
> >> the call to CodeBuffer::copy_code_and_locs_to() from the CodeBlob
> >> constructor into the nmethod and RuntimeBlob constructors
> >> respectively. I couldn't find a reason why we shouldn't do this. The
> >> change is minimal and makes the whole handling more robust. I've
> >> compiled and smoke tested with JVM 98 on Linux/x86_64, Linux/ppc64le
> >> and Solaris/SPARC.
> >>
> >> I will run some more tests on Monday, but it would be great if you
> >> (i.e. Andrew) could verify this fix on ARM and if you (i.e.
> >> Rickard/Dean) could run some of your internal tests.
> >
> >
> > Hi Volker. This looks like what I was trying to accomplish with my patch,
> > but I introduced a new function CodeBlob::initialize(), and I called it too
> > early (in CompiledMethod instead of nmethod).
> >
>
> Yes, now I see. Initially I just patched your changes in and saw that
> they don't work so I didn't looked at the actual changes in more
> detail. I think I'll add the various casts/type changes from your
> version (they are a nice clean-up) into my patch if Rickard agrees to
> fix this independently of his change.
>
> >> I'd also like to ask if I should submit an extra RFR for 8151956 with
> >> my fix or if we should close 8151956 and fix it as part of Rickard's
> >> change for 8152664. I'd be happy with both solutions :)
> >
> >
> > Either way is fine with me.
> >
>
> OK, let's wait for Rickard's opinion.
>
> > dl
> >
> >
> >> A nice weekend everybody,
> >> Volker
> >>
> >>
> >> On Wed, Apr 13, 2016 at 3:31 PM, Rickard Bäckman
> >> <rickard.backman at oracle.com> wrote:
> >>>
> >>> Volker,
> >>>
> >>> yes, I didn't realize at first that the nmethod was casted to a
> >>> CompiledMethod before the call to consts_begin(). Otherwise it would
> >>> have used the non-virtual consts_begin of nmethod that didn't have any
> >>> virtual calls.
> >>>
> >>> The entire code chain and looking up itself from the CodeCache before
> >>> fully constructed seems quite problematic. Even before the changes I
> >>> made. Previous to my changes the calls would have succeeded but returned
> >>> header_begin() or this for all the consts_begin, consts_end, etc... ?
> >>>
> >>> /R
> >>>
> >>> On 04/11, Volker Simonis wrote:
> >>>>
> >>>> Rickard, Dean,
> >>>>
> >>>> I'm afraid all this hacks can not work. It doesn't help to make
> >>>> CompiledMethod::consts_begin() non-virtual and then calling a virtual
> >>>> function from it. The problem ist that at the point where you call
> >>>> consts_begin_v(), the vtable of 'this' is still the one of CodeBlob and
> >>>> this results in calling yet another arbitrary function:
> >>>>
> >>>> #0 CodeBlob::is_locked_by_vm (this=0x3fff607d0c10) at
> >>>>
> >>>> /usr/work/d046063/OpenJDK/jdk9-hs-comp/hotspot/src/share/vm/code/codeBlob.hpp:168
> >>>> #1 0x00003fffb6e38048 in CompiledMethod::consts_begin
> >>>> (this=0x3fff607d0c10) at
> >>>>
> >>>> /usr/work/d046063/OpenJDK/jdk9-hs-comp/hotspot/src/share/vm/code/compiledMethod.hpp:255
> >>>> #2 0x00003fffb758d658 in RelocIterator::initialize
> >>>> (this=0x3ffdfd3fc9a8,
> >>>> nm=0x3fff607d0c10, begin=0x3fff607d0fac "\001", limit=0x0) at
> >>>>
> >>>> /usr/work/d046063/OpenJDK/jdk9-hs-comp/hotspot/src/share/vm/code/relocInfo.cpp:142
> >>>> #3 0x00003fffb6ace56c in RelocIterator::RelocIterator
> >>>> (this=0x3ffdfd3fc9a8, nm=0x3fff607d0c10, begin=0x3fff607d0fac "\001",
> >>>> limit=0x0) at
> >>>>
> >>>> /usr/work/d046063/OpenJDK/jdk9-hs-comp/hotspot/src/share/vm/code/relocInfo.hpp:1378
> >>>> #4 0x00003fffb7591afc in trampoline_stub_Relocation::get_trampoline_for
> >>>> (call=0x3fff607d0fac "\001", code=0x3fff607d0c10) at
> >>>>
> >>>> /usr/work/d046063/OpenJDK/jdk9-hs-comp/hotspot/src/share/vm/code/relocInfo.cpp:849
> >>>> #5 0x00003fffb741ba4c in NativeCall::get_trampoline
> >>>> (this=0x3fff607d0fac)
> >>>> at
> >>>>
> >>>> /usr/work/d046063/OpenJDK/jdk9-hs-comp/hotspot/src/cpu/ppc/vm/nativeInst_ppc.cpp:146
> >>>> #6 0x00003fffb7596a34 in Relocation::pd_call_destination
> >>>> (this=0x3ffdfd3fcd10, orig_addr=0x3fff6033482c "\001") at
> >>>>
> >>>> /usr/work/d046063/OpenJDK/jdk9-hs-comp/hotspot/src/cpu/ppc/vm/relocInfo_ppc.cpp:87
> >>>> #7 0x00003fffb758f71c in CallRelocation::fix_relocation_after_move
> >>>> (this=0x3ffdfd3fcd10, src=0x3ffdfd3fdbc0, dest=0x3ffdfd3fcdd8) at
> >>>>
> >>>> /usr/work/d046063/OpenJDK/jdk9-hs-comp/hotspot/src/share/vm/code/relocInfo.cpp:506
> >>>> #8 0x00003fffb6c48914 in CodeBuffer::relocate_code_to
> >>>> (this=0x3ffdfd3fdbc0, dest=0x3ffdfd3fcdd8) at
> >>>>
> >>>> /usr/work/d046063/OpenJDK/jdk9-hs-comp/hotspot/src/share/vm/asm/codeBuffer.cpp:812
> >>>> #9 0x00003fffb6c48480 in CodeBuffer::copy_code_to (this=0x3ffdfd3fdbc0,
> >>>> dest_blob=0x3fff607d0c10) at
> >>>>
> >>>> /usr/work/d046063/OpenJDK/jdk9-hs-comp/hotspot/src/share/vm/asm/codeBuffer.cpp:748
> >>>> #10 0x00003fffb6c426ec in CodeBuffer::copy_code_and_locs_to
> >>>> (this=0x3ffdfd3fdbc0, blob=0x3fff607d0c10) at
> >>>>
> >>>> /usr/work/d046063/OpenJDK/jdk9-hs-comp/hotspot/src/share/vm/asm/codeBuffer.hpp:607
> >>>> #11 0x00003fffb6c3f8b0 in CodeBlob::CodeBlob (this=0x3fff607d0c10,
> >>>> name=0x3fffb7a760f8 "nmethod", layout=..., cb=0x3ffdfd3fdbc0,
> >>>> frame_complete_offset=20, frame_size=14, oop_maps=0x3ffe0001ed00,
> >>>> caller_must_gc_arguments=false, subtype=8) at
> >>>>
> >>>> /usr/work/d046063/OpenJDK/jdk9-hs-comp/hotspot/src/share/vm/code/codeBlob.cpp:117
> >>>> #12 0x00003fffb6ce5360 in CompiledMethod::CompiledMethod
> >>>> (this=0x3fff607d0c10, method=0x3ffe1ddce568, name=0x3fffb7a760f8
> >>>> "nmethod",
> >>>> size=1768, header_size=392, cb=0x3ffdfd3fdbc0, frame_complete_offset=20,
> >>>> frame_size=14, oop_maps=0x3ffe0001ed00, caller_must_gc_arguments=false)
> >>>> at
> >>>>
> >>>> /usr/work/d046063/OpenJDK/jdk9-hs-comp/hotspot/src/share/vm/code/compiledMethod.cpp:42
> >>>> #13 0x00003fffb7422198 in nmethod::nmethod (this=0x3fff607d0c10,
> >>>> method=0x3ffe1ddce568, nmethod_size=1768, compile_id=4, entry_bci=-1,
> >>>> offsets=0x3ffdfd3fdb98, orig_pc_offset=104, debug_info=0x3fffb03f2dc0,
> >>>> dependencies=0x3ffe0001ed70, code_buffer=0x3ffdfd3fdbc0, frame_size=14,
> >>>> oop_maps=0x3ffe0001ed00, handler_table=0x3ffdfd3fdb50,
> >>>> nul_chk_table=0x3ffdfd3fdb70, compiler=0x3fffb03d0cd0, comp_level=3) at
> >>>>
> >>>> /usr/work/d046063/OpenJDK/jdk9-hs-comp/hotspot/src/share/vm/code/nmethod.cpp:706
> >>>> #14 0x00003fffb7421850 in nmethod::new_nmethod (method=...,
> >>>> compile_id=4,
> >>>> entry_bci=-1, offsets=0x3ffdfd3fdb98, orig_pc_offset=104,
> >>>> debug_info=0x3fffb03f2dc0, dependencies=0x3ffe0001ed70,
> >>>> code_buffer=0x3ffdfd3fdbc0, frame_size=14, oop_maps=0x3ffe0001ed00,
> >>>> handler_table=0x3ffdfd3fdb50, nul_chk_table=0x3ffdfd3fdb70,
> >>>> compiler=0x3fffb03d0cd0, comp_level=3) at
> >>>>
> >>>> /usr/work/d046063/OpenJDK/jdk9-hs-comp/hotspot/src/share/vm/code/nmethod.cpp:548
> >>>>
> >>>> I think we really need to rework this as proposed by Andrew in his last
> >>>> mail. I'm working on such a fix.
> >>>>
> >>>> Regards,
> >>>> Volker
> >>>>
> >>>>
> >>>> On Mon, Apr 11, 2016 at 1:55 PM, Rickard Bäckman
> >>>> <rickard.backman at oracle.com
> >>>>>
> >>>>> wrote:
> >>>>> Volker,
> >>>>>
> >>>>> here is the patch if you want to try it.
> >>>>> http://cr.openjdk.java.net/~rbackman/8152664/virtual.patch
> >>>>>
> >>>>> /R
> >>>>>
> >>>>> On 04/11, Rickard Bäckman wrote:
> >>>>>>
> >>>>>> Volker,
> >>>>>>
> >>>>>> thanks for finding this issue.
> >>>>>>
> >>>>>> I think that maybe the easiest fix is as follows:
> >>>>>>
> >>>>>> create new virtual methods in CompiledMethod:
> >>>>>>
> >>>>>> virtual address stub_begin_v() = 0;
> >>>>>>
> >>>>>> make the now virtual stub_begin non-virtual like:
> >>>>>>
> >>>>>> address stub_begin() { return stub_begin_v(); }
> >>>>>>
> >>>>>> in nmethod we override the stub_begin() with the normal this + offset
> >>>>>> compuation and implement stub_begin_v() to call stub_begin().
> >>>>>>
> >>>>>> That will avoid all virtual calls in the case were we are not working
> >>>>>> on
> >>>>>> a CompiledMethod.
> >>>>>>
> >>>>>> It adds a couple of methods though. What do you think?
> >>>>>>
> >>>>>> /R
> >>>>>>
> >>>>>> On 04/08, Volker Simonis wrote:
> >>>>>>>
> >>>>>>> Hi Rickard,
> >>>>>>>
> >>>>>>> I found the problem why your change crashes the VM on ppc (and I'm
> >>>>>
> >>>>> pretty
> >>>>>>>
> >>>>>>> sure it will also crash on ARM - @Andrew, maybe you can try it out?).
> >>>>>
> >>>>> It is
> >>>>>>>
> >>>>>>> caused by the following code in address NativeCall::get_trampoline()
> >>>>>
> >>>>> which
> >>>>>>>
> >>>>>>> is also present on arm64:
> >>>>>>>
> >>>>>>> address NativeCall::get_trampoline() {
> >>>>>>> address call_addr = addr_at(0);
> >>>>>>> CodeBlob *code = CodeCache::find_blob(call_addr);
> >>>>>>> ...
> >>>>>>> // If the codeBlob is not a nmethod, this is because we get here
> >>>>>
> >>>>> from the
> >>>>>>>
> >>>>>>> // CodeBlob constructor, which is called within the nmethod
> >>>>>
> >>>>> constructor.
> >>>>>>>
> >>>>>>> return trampoline_stub_Relocation::get_trampoline_for(call_addr,
> >>>>>>> (nmethod*)code);
> >>>>>>> }
> >>>>>>>
> >>>>>>> The comment explains the situation quite well: we're in the CodeBlob
> >>>>>>> constructor which was called by the CompiledMethod constructor which
> >>>>>
> >>>>> was
> >>>>>>>
> >>>>>>> called from the nmethod constructor:
> >>>>>>>
> >>>>>>> #3 0x00003fffb741b80c in NativeCall::get_trampoline
> >>>>>
> >>>>> (this=0x3fff607d0fac)
> >>>>>>>
> >>>>>>> at
> >>>>>>>
> >>>>>
> >>>>> /usr/work/d046063/OpenJDK/jdk9-hs-comp/hotspot/src/cpu/ppc/vm/nativeInst_ppc.cpp:146
> >>>>>>>
> >>>>>>> #4 0x00003fffb7596914 in Relocation::pd_call_destination
> >>>>>>> (this=0x3ffdfe3fcc90, orig_addr=0x3fff603b8a2c "\001") at
> >>>>>>>
> >>>>>
> >>>>> /usr/work/d046063/OpenJDK/jdk9-hs-comp/hotspot/src/cpu/ppc/vm/relocInfo_ppc.cpp:87
> >>>>>>>
> >>>>>>> #5 0x00003fffb758f5fc in CallRelocation::fix_relocation_after_move
> >>>>>>> (this=0x3ffdfe3fcc90, src=0x3ffdfe3fdb40, dest=0x3ffdfe3fcd58) at
> >>>>>>>
> >>>>>
> >>>>> /usr/work/d046063/OpenJDK/jdk9-hs-comp/hotspot/src/share/vm/code/relocInfo.cpp:506
> >>>>>>>
> >>>>>>> #6 0x00003fffb6c48898 in CodeBuffer::relocate_code_to
> >>>>>>> (this=0x3ffdfe3fdb40, dest=0x3ffdfe3fcd58) at
> >>>>>>>
> >>>>>
> >>>>> /usr/work/d046063/OpenJDK/jdk9-hs-comp/hotspot/src/share/vm/asm/codeBuffer.cpp:812
> >>>>>>>
> >>>>>>> #7 0x00003fffb6c48404 in CodeBuffer::copy_code_to
> >>>>>
> >>>>> (this=0x3ffdfe3fdb40,
> >>>>>>>
> >>>>>>> dest_blob=0x3fff607d0c10) at
> >>>>>>>
> >>>>>
> >>>>> /usr/work/d046063/OpenJDK/jdk9-hs-comp/hotspot/src/share/vm/asm/codeBuffer.cpp:748
> >>>>>>>
> >>>>>>> #8 0x00003fffb6c42670 in CodeBuffer::copy_code_and_locs_to
> >>>>>>> (this=0x3ffdfe3fdb40, blob=0x3fff607d0c10) at
> >>>>>>>
> >>>>>
> >>>>> /usr/work/d046063/OpenJDK/jdk9-hs-comp/hotspot/src/share/vm/asm/codeBuffer.hpp:607
> >>>>>>>
> >>>>>>> #9 0x00003fffb6c3f834 in CodeBlob::CodeBlob (this=0x3fff607d0c10,
> >>>>>>> name=0x3fffb7a75fd8 "nmethod", layout=..., cb=0x3ffdfe3fdb40,
> >>>>>>> frame_complete_offset=20, frame_size=14, oop_maps=0x3ffe00049620,
> >>>>>>> caller_must_gc_arguments=false, subtype=8) at
> >>>>>>>
> >>>>>
> >>>>> /usr/work/d046063/OpenJDK/jdk9-hs-comp/hotspot/src/share/vm/code/codeBlob.cpp:117
> >>>>>>>
> >>>>>>> #10 0x00003fffb6ce52c8 in CompiledMethod::CompiledMethod
> >>>>>>> (this=0x3fff607d0c10, method=0x3ffe1ddce568, name=0x3fffb7a75fd8
> >>>>>
> >>>>> "nmethod",
> >>>>>>>
> >>>>>>> size=1768, header_size=392, cb=0x3ffdfe3fdb40,
> >>>>>
> >>>>> frame_complete_offset=20,
> >>>>>>>
> >>>>>>> frame_size=14, oop_maps=0x3ffe00049620,
> >>>>>
> >>>>> caller_must_gc_arguments=false) at
> >>>>>
> >>>>> /usr/work/d046063/OpenJDK/jdk9-hs-comp/hotspot/src/share/vm/code/compiledMethod.cpp:42
> >>>>>>>
> >>>>>>> #11 0x00003fffb7421f58 in nmethod::nmethod (this=0x3fff607d0c10,
> >>>>>>> method=0x3ffe1ddce568, nmethod_size=1768, compile_id=4, entry_bci=-1,
> >>>>>>> offsets=0x3ffdfe3fdb18, orig_pc_offset=104,
> >>>>>>> debug_info=0x3fffb03d55f0,
> >>>>>>> dependencies=0x3ffe00049690, code_buffer=0x3ffdfe3fdb40,
> >>>>>>> frame_size=14,
> >>>>>>> oop_maps=0x3ffe00049620, handler_table=0x3ffdfe3fdad0,
> >>>>>>> nul_chk_table=0x3ffdfe3fdaf0, compiler=0x3fffb03bc270, comp_level=3)
> >>>>>>> at
> >>>>>>>
> >>>>>
> >>>>> /usr/work/d046063/OpenJDK/jdk9-hs-comp/hotspot/src/share/vm/code/nmethod.cpp:706
> >>>>>>>
> >>>>>>> Now we cast 'code' to 'nmethod' but at this point in time 'code' is
> >>>>>
> >>>>> still a
> >>>>>>>
> >>>>>>> CodeBlob from the C++ point of view (i.e. it still has a CodeBlob
> >>>>>
> >>>>> vtable
> >>>>>>>
> >>>>>>> (see [1] for an explanation)).
> >>>>>>>
> >>>>>>> Later on, in RelocIterator::initialize() we call virtual methods on
> >>>>>>> the
> >>>>>>> nmethod which still has the vtable of a "CodeBlob" and this fails
> >>>>>
> >>>>> badly:
> >>>>>>>
> >>>>>>> #0 SingletonBlob::print_on (this=0x3fff607d0c10, st=0x0) at
> >>>>>>>
> >>>>>
> >>>>> /usr/work/d046063/OpenJDK/jdk9-hs-comp/hotspot/src/share/vm/code/codeBlob.cpp:584
> >>>>>>>
> >>>>>>> #1 0x00003fffb758d51c in RelocIterator::initialize
> >>>>>
> >>>>> (this=0x3ffdfe3fc928,
> >>>>>>>
> >>>>>>> nm=0x3fff607d0c10, begin=0x3fff607d0fac "\001", limit=0x0) at
> >>>>>>>
> >>>>>
> >>>>> /usr/work/d046063/OpenJDK/jdk9-hs-comp/hotspot/src/share/vm/code/relocInfo.cpp:144
> >>>>>>>
> >>>>>>> #2 0x00003fffb6ace56c in RelocIterator::RelocIterator
> >>>>>>> (this=0x3ffdfe3fc928, nm=0x3fff607d0c10, begin=0x3fff607d0fac "\001",
> >>>>>>> limit=0x0) at
> >>>>>>>
> >>>>>
> >>>>> /usr/work/d046063/OpenJDK/jdk9-hs-comp/hotspot/src/share/vm/code/relocInfo.hpp:1378
> >>>>>>>
> >>>>>>> #3 0x00003fffb75919dc in
> >>>>>
> >>>>> trampoline_stub_Relocation::get_trampoline_for
> >>>>>>>
> >>>>>>> (call=0x3fff607d0fac "\001", code=0x3fff607d0c10) at
> >>>>>>>
> >>>>>
> >>>>> /usr/work/d046063/OpenJDK/jdk9-hs-comp/hotspot/src/share/vm/code/relocInfo.cpp:849
> >>>>>>>
> >>>>>>> #4 0x00003fffb741b80c in NativeCall::get_trampoline
> >>>>>
> >>>>> (this=0x3fff607d0fac)
> >>>>>>>
> >>>>>>> at
> >>>>>>>
> >>>>>
> >>>>> /usr/work/d046063/OpenJDK/jdk9-hs-comp/hotspot/src/cpu/ppc/vm/nativeInst_ppc.cpp:146
> >>>>>>>
> >>>>>>> As you can see, we actually want to call nmethod::stub_begin() at
> >>>>>>> relocInfo.cpp:144
> >>>>>>>
> >>>>>>> 142 _section_start[CodeBuffer::SECT_CONSTS] = nm->consts_begin();
> >>>>>>> 143 _section_start[CodeBuffer::SECT_INSTS ] = nm->insts_begin() ;
> >>>>>>> 144 _section_start[CodeBuffer::SECT_STUBS ] = nm->stub_begin() ;
> >>>>>>>
> >>>>>>> but we actually end up in SingletonBlob::print_on() which is a
> >>>>>
> >>>>> completely
> >>>>>>>
> >>>>>>> different method. Notice that the call to nm->consts_begin() before
> >>>>>
> >>>>> also
> >>>>>>>
> >>>>>>> fails, but it doesn't crash the VM because it happens to call
> >>>>>>> SingletonBlob::verify() which has no bad side effect. The call to
> >>>>>>> nm->insts_begin() in line 143 is non-virtual and thus works fine.
> >>>>>>> Here
> >>>>>
> >>>>> are
> >>>>>>>
> >>>>>>> the corresponding vtable slots in the CodeBlob vtable for
> >>>>>
> >>>>> consts_begin()
> >>>>>>>
> >>>>>>> and stub_begin()
> >>>>>>>
> >>>>>>> (gdb) p &nmethod::consts_begin
> >>>>>>> $76 = &virtual table offset 42
> >>>>>>> (gdb) p &nmethod::stub_begin
> >>>>>>> $77 = &virtual table offset 44
> >>>>>>> (gdb) p ((*(void ***)nm) + 1)[42]
> >>>>>>> $86 = (void *) 0x3fffb6c41df8 <SingletonBlob::verify()>
> >>>>>>> (gdb) p ((*(void ***)nm) + 1)[44]
> >>>>>>> $87 = (void *) 0x3fffb6c41e64 <SingletonBlob::print_on(outputStream*)
> >>>>>
> >>>>> const>
> >>>>>>>
> >>>>>>> As you can see, 'nm' is indeed a "CodeBlob" at this point:
> >>>>>>>
> >>>>>>> (gdb) p *(void ***)nm
> >>>>>>> $91 = (void **) 0x3fffb7befa00 <vtable for CodeBlob+16>
> >>>>>>> (gdb) p nm->print()
> >>>>>>> [CodeBlob (0x00003fff607d1090)]
> >>>>>>> Framesize: 14
> >>>>>>>
> >>>>>>> The offending calls succeeded before your change, because they where
> >>>>>
> >>>>> not
> >>>>>>>
> >>>>>>> virtual. Any idea how we can fix this with the new class hierarchy?
> >>>>>>>
> >>>>>>> Regards,
> >>>>>>> Volker
> >>>>>>>
> >>>>>>> [1]
> >>>>>>>
> >>>>>
> >>>>> http://stackoverflow.com/questions/6591859/when-does-the-vptr-pointing-to-vtable-get-initialized-for-a-polymorphic-class
> >>>>>>>
> >>>>>>>
> >>>>>>>
> >>>>>>> On Thu, Apr 7, 2016 at 5:50 PM, Volker Simonis <
> >>>>>
> >>>>> volker.simonis at gmail.com>
> >>>>>>>
> >>>>>>> wrote:
> >>>>>>>
> >>>>>>>> Hi Rickard,
> >>>>>>>>
> >>>>>>>> I'd also like to know what's the rational behind this quite large
> >>>>>>>> change. Do you expect some performance or memory consumption
> >>>>>>>> improvements or is this a prerequisite for another change which is
> >>>>>>>> still to come?
> >>>>>>>>
> >>>>>>>> The change itself currently doesn't work on ppc64 (neither on Linux
> >>>>>>>> nor on AIX). I get the following crash during the build when the
> >>>>>
> >>>>> newly
> >>>>>>>>
> >>>>>>>> built Hotspot is JIT-compiling java.lang.String::charAt on C1 :
> >>>>>>>>
> >>>>>>>> #
> >>>>>>>> # A fatal error has been detected by the Java Runtime Environment:
> >>>>>>>> #
> >>>>>>>> # SIGSEGV (0xb) at pc=0x00001000012a44d0, pid=35331, tid=35404
> >>>>>>>> #
> >>>>>>>> # JRE version: OpenJDK Runtime Environment (9.0) (slowdebug build
> >>>>>>>> 9-internal+0-2016-04-07-162501.d046063.jdk9-hs-comp)
> >>>>>>>> # Java VM: OpenJDK 64-Bit Server VM (slowdebug
> >>>>>>>> 9-internal+0-2016-04-07-162501.d046063.jdk9-hs-comp, mixed mode,
> >>>>>>>> tiered, compressed oo
> >>>>>>>> ps, serial gc, linux-ppc64le)
> >>>>>>>> # Problematic frame:
> >>>>>>>> # V [libjvm.so+0xf744d0] outputStream::do_vsnprintf_and_write(char
> >>>>>>>> const*, char*, bool)+0x40
> >>>>>>>> #
> >>>>>>>> # No core dump will be written. Core dumps have been disabled. To
> >>>>>>>> enable core dumping, try "ulimit -c unlimited" before starting Java
> >>>>>>>> again
> >>>>>>>> #
> >>>>>>>> # If you would like to submit a bug report, please visit:
> >>>>>>>> # http://bugreport.java.com/bugreport/crash.jsp
> >>>>>>>> #
> >>>>>>>>
> >>>>>>>> --------------- S U M M A R Y ------------
> >>>>>>>>
> >>>>>>>> Command Line:
> >>>>>>>>
> >>>>> -Dapplication.home=/sapmnt/ld9510/a/d046063/output-jdk9-hs-comp-dbg/jdk
> >>>>>>>>
> >>>>>>>> -Xms8m -XX:+UseSerialGC -Xms32M -Xmx512M -Djdk.
> >>>>>>>> module.main=jdk.jlink jdk.jlink/jdk.tools.jmod.Main create
> >>>>>>>> --module-version 9-internal --os-name Linux --os-arch ppc64le
> >>>>>>>> --os-version
> >>>>>>>> 2.6 --modulepath
> >>>>>>>> /priv/d046063/output-jdk9-hs-comp-dbg/images/jmods
> >>>>>>>> --hash-dependencies .* --exclude **_the.* --libs
> >>>>>>>>
> >>>>>>>>
> >>>>>
> >>>>> /priv/d046063/output-jdk9-hs-comp-dbg/support/modules_libs-stripped/java.base
> >>>>>>>>
> >>>>>>>> --cmds
> >>>>>>>>
> >>>>>
> >>>>> /priv/d046063/output-jdk9-hs-comp-dbg/support/modules_cmds-stripped/java.base
> >>>>>>>>
> >>>>>>>> --config
> >>>>>>>> /priv/d046063/output-jdk9-hs-comp-dbg/support/modules_conf/java.base
> >>>>>>>> --class-path
> >>>>>
> >>>>> /priv/d046063/output-jdk9-hs-comp-dbg/jdk/modules/java.base
> >>>>>>>>
> >>>>>>>> /priv/d046063/output-jdk9-hs-comp-dbg/support/jmods/java.base.jmod
> >>>>>>>>
> >>>>>>>> Host: ld9510, POWER8E (raw), altivec supported, 48 cores, 61G, #
> >>>>>>>> Please check /etc/os-release for details about this release.
> >>>>>>>> Time: Thu Apr 7 16:28:55 2016 CEST elapsed time: 0 seconds (0d 0h
> >>>>>
> >>>>> 0m 0s)
> >>>>>>>>
> >>>>>>>> --------------- T H R E A D ---------------
> >>>>>>>>
> >>>>>>>> Current thread (0x000010000429c800): JavaThread "C1
> >>>>>
> >>>>> CompilerThread10"
> >>>>>>>>
> >>>>>>>> daemon [_thread_in_vm, id=35404,
> >>>>>>>> stack(0x000010006a800000,0x000010006ac00000)]
> >>>>>>>>
> >>>>>>>>
> >>>>>>>> Current CompileTask:
> >>>>>>>> C1: 761 3 3 java.lang.String::charAt (25 bytes)
> >>>>>>>>
> >>>>>>>> Stack: [0x000010006a800000,0x000010006ac00000],
> >>>>>>>> sp=0x000010006abfc6c0, free space=4081k
> >>>>>>>> Native frames: (J=compiled Java code, j=interpreted, Vv=VM code,
> >>>>>
> >>>>> C=native
> >>>>>>>>
> >>>>>>>> code)
> >>>>>>>> V [libjvm.so+0xf744d0] outputStream::do_vsnprintf_and_write(char
> >>>>>>>> const*, char*, bool)+0x40
> >>>>>>>> V [libjvm.so+0xf74668] outputStream::print_cr(char const*,
> >>>>>
> >>>>> ...)+0x68
> >>>>>>>>
> >>>>>>>> V [libjvm.so+0x72189c] CodeBlob::print_on(outputStream*)
> >>>>>>>> const+0x50
> >>>>>>>> V [libjvm.so+0x723bdc] RuntimeBlob::print_on(outputStream*)
> >>>>>
> >>>>> const+0x40
> >>>>>>>>
> >>>>>>>> V [libjvm.so+0x721eb0] SingletonBlob::print_on(outputStream*)
> >>>>>
> >>>>> const+0x4c
> >>>>>>>>
> >>>>>>>> V [libjvm.so+0x106d51c] RelocIterator::initialize(CompiledMethod*,
> >>>>>>>> unsigned char*, unsigned char*)+0x170
> >>>>>>>> V [libjvm.so+0x5ae56c]
> >>>>>
> >>>>> RelocIterator::RelocIterator(CompiledMethod*,
> >>>>>>>>
> >>>>>>>> unsigned char*, unsigned char*)+0x78
> >>>>>>>> V [libjvm.so+0x10719dc]
> >>>>>>>> trampoline_stub_Relocation::get_trampoline_for(unsigned char*,
> >>>>>>>> nmethod*)+0x78
> >>>>>>>> V [libjvm.so+0xefb80c] NativeCall::get_trampoline()+0x110
> >>>>>>>> V [libjvm.so+0x1076914] Relocation::pd_call_destination(unsigned
> >>>>>>>> char*)+0x150
> >>>>>>>> V [libjvm.so+0x106f5fc]
> >>>>>>>> CallRelocation::fix_relocation_after_move(CodeBuffer const*,
> >>>>>>>> CodeBuffer*)+0x74
> >>>>>>>> V [libjvm.so+0x728898] CodeBuffer::relocate_code_to(CodeBuffer*)
> >>>>>>>> const+0x390
> >>>>>>>> V [libjvm.so+0x728404] CodeBuffer::copy_code_to(CodeBlob*)+0x134
> >>>>>>>> V [libjvm.so+0x722670]
> >>>>>
> >>>>> CodeBuffer::copy_code_and_locs_to(CodeBlob*)+0x84
> >>>>>>>>
> >>>>>>>> V [libjvm.so+0x71f834] CodeBlob::CodeBlob(char const*,
> >>>>>>>> CodeBlobLayout const&, CodeBuffer*, int, int, OopMapSet*, bool,
> >>>>>>>> int)+0x320
> >>>>>>>> V [libjvm.so+0x7c52c8] CompiledMethod::CompiledMethod(Method*,
> >>>>>>>> char
> >>>>>>>> const*, int, int, CodeBuffer*, int, int, OopMapSet*, bool)+0xd8
> >>>>>>>> V [libjvm.so+0xf01f58] nmethod::nmethod(Method*, int, int, int,
> >>>>>>>> CodeOffsets*, int, DebugInformationRecorder*, Dependencies*,
> >>>>>>>> CodeBuffer*, int, OopMapSet*, ExceptionHandlerTable*,
> >>>>>>>> ImplicitExceptionTable*, AbstractCompiler*, int)+0xe0
> >>>>>>>> V [libjvm.so+0xf01610] nmethod::new_nmethod(methodHandle const&,
> >>>>>>>> int, int, CodeOffsets*, int, DebugInformationRecorder*,
> >>>>>
> >>>>> Dependencies*,
> >>>>>>>>
> >>>>>>>> CodeBuffer*, int, OopMapSet*, ExceptionHandlerTable*,
> >>>>>>>> ImplicitExceptionTable*, AbstractCompiler*, int)+0x2c4
> >>>>>>>> V [libjvm.so+0x632970] ciEnv::register_method(ciMethod*, int,
> >>>>>>>> CodeOffsets*, int, CodeBuffer*, int, OopMapSet*,
> >>>>>>>> ExceptionHandlerTable*, ImplicitExceptionTable*, AbstractCompiler*,
> >>>>>>>> bool, bool, RTMState)+0x560
> >>>>>>>> V [libjvm.so+0x48ee00] Compilation::install_code(int)+0x264
> >>>>>>>> V [libjvm.so+0x48eff8] Compilation::compile_method()+0x184
> >>>>>>>> V [libjvm.so+0x48f7a8] Compilation::Compilation(AbstractCompiler*,
> >>>>>>>> ciEnv*, ciMethod*, int, BufferBlob*, DirectiveSet*)+0x288
> >>>>>>>> V [libjvm.so+0x4980d0] Compiler::compile_method(ciEnv*, ciMethod*,
> >>>>>>>> int, DirectiveSet*)+0xc8
> >>>>>>>> V [libjvm.so+0x7b188c]
> >>>>>>>> CompileBroker::invoke_compiler_on_method(CompileTask*)+0x590
> >>>>>>>> V [libjvm.so+0x7b07bc] CompileBroker::compiler_thread_loop()+0x310
> >>>>>>>> V [libjvm.so+0x11a614c] compiler_thread_entry(JavaThread*,
> >>>>>
> >>>>> Thread*)+0xa0
> >>>>>>>>
> >>>>>>>> V [libjvm.so+0x119f3a8] JavaThread::thread_main_inner()+0x1b4
> >>>>>>>> V [libjvm.so+0x119f1a4] JavaThread::run()+0x1b8
> >>>>>>>> V [libjvm.so+0xf53d90] java_start(Thread*)+0x204
> >>>>>>>> C [libpthread.so.0+0x8a64] start_thread+0xf4
> >>>>>>>> C [libc.so.6+0x1032a0] clone+0x98
> >>>>>>>>
> >>>>>>>> I haven't identified the exact cause (will analyze it tomorrow) but
> >>>>>>>> the stack trace indicates that it is indeed related to your changes.
> >>>>>>>>
> >>>>>>>> Besides that I have some comments:
> >>>>>>>>
> >>>>>>>> codeBuffer.hpp:
> >>>>>>>>
> >>>>>>>> 472 CodeSection* insts() { return &_insts; }
> >>>>>>>> 475 const CodeSection* insts() const { return &_insts; }
> >>>>>>>>
> >>>>>>>> - do we really need both versions?
> >>>>>>>>
> >>>>>>>> codeBlob.hpp:
> >>>>>>>>
> >>>>>>>> 135 nmethod* as_nmethod_or_null() const { return
> >>>>>>>> is_nmethod() ? (nmethod*) this : NULL; }
> >>>>>>>> 136 nmethod* as_nmethod() const {
> >>>>>>>> assert(is_nmethod(), "must be nmethod"); return (nmethod*) this; }
> >>>>>>>> 137 CompiledMethod* as_compiled_method_or_null() const { return
> >>>>>>>> is_compiled() ? (CompiledMethod*) this : NULL; }
> >>>>>>>> 138 CompiledMethod* as_compiled_method() const {
> >>>>>>>> assert(is_compiled(), "must be compiled"); return (CompiledMethod*)
> >>>>>>>> this; }
> >>>>>>>> 139 CodeBlob* as_codeblob_or_null() const { return
> >>>>>>>> (CodeBlob*) this; }
> >>>>>>>>
> >>>>>>>> - I don't like this code. You make the getters 'const' which
> >>>>>>>> implicitely makes 'this' a "pointer to const" but then the returned
> >>>>>>>> pointer is a normal pointer to a non-const object and therefore you
> >>>>>>>> have to statically cast away the "pointer to const" (that's why you
> >>>>>>>> need the cast even in the case where you return a CodeBlob*). So
> >>>>>>>> either remove the const qualifier from the method declarations or
> >>>>>
> >>>>> make
> >>>>>>>>
> >>>>>>>> them return "pointers to const". And by the way,
> >>>>>
> >>>>> as_codeblob_or_null()
> >>>>>>>>
> >>>>>>>> doesn't seemed to be used anywhere in the code, why do we need it at
> >>>>>>>> all?
> >>>>>>>>
> >>>>>>>> - Why do we need the non-virtual methods is_nmethod() and
> >>>>>>>> is_compiled() to manually simulate virtual behavior. Why can't we
> >>>>>>>> simply make them virtual and implement them accordingly in nmathod
> >>>>>
> >>>>> and
> >>>>>>>>
> >>>>>>>> CompiledMethod?
> >>>>>>>>
> >>>>>>>> Regards,
> >>>>>>>> Volker
> >>>>>>>>
> >>>>>>>> On Thu, Apr 7, 2016 at 2:12 PM, Rickard Bäckman
> >>>>>>>> <rickard.backman at oracle.com> wrote:
> >>>>>>>>>
> >>>>>>>>> Hi,
> >>>>>>>>>
> >>>>>>>>> can I please have review for this patch please?
> >>>>>>>>>
> >>>>>>>>> So far CodeBlobs have required all the data (metadata, oops, code,
> >>>>>
> >>>>> etc)
> >>>>>>>>>
> >>>>>>>>> to be in one continuous blob With this patch we are looking to
> >>>>>
> >>>>> change
> >>>>>>>>>
> >>>>>>>>> that. It's been done by changing offsets in CodeBlob to addresses,
> >>>>>>>>> making some methods virtual to allow different behavior and also
> >>>>>>>>> creating a couple of new classes. CompiledMethod now sits inbetween
> >>>>>>>>> CodeBlob and nmethod.
> >>>>>>>>>
> >>>>>>>>> CR: https://bugs.openjdk.java.net/browse/JDK-8152664
> >>>>>>>>> Webrev: http://cr.openjdk.java.net/~rbackman/8152664/
> >>>>>>>>>
> >>>>>>>>> Thanks
> >>>>>>>>> /R
> >
> >
More information about the hotspot-dev
mailing list