Hi Serguei, thanks for sponsoring!
So just waiting for another reviewer. Anybody volunteers:) Regards, Volker On Tue, Jul 15, 2014 at 11:09 AM, serguei.spit...@oracle.com <serguei.spit...@oracle.com> wrote: > On 7/15/14 1:48 AM, Volker Simonis wrote: > > On Mon, Jul 14, 2014 at 10:35 PM, serguei.spit...@oracle.com > <serguei.spit...@oracle.com> wrote: > > Hi Volker, > > It looks good in general. > > But I don't understand all the details. > For instance, your email description of the fix tells that the the event is > posted by: > > RuntimeStub::new_runtime_stub() -> CodeBlob::trace_new_stub() -> > JvmtiExport::post_dynamic_code_generated() > > I see the new_runtime_stub() call in the generate_throw_exception() but > there is no such call > in the generate_icache_flush() and generate_handler_for_unsafe_access() . > > Probably, the StubCodeMark just needs to be removed there. > Could you, please, explain this a little bit? > > Hi Serguei, > > Thank you for looking at my change. I tried to explain your questions > in my initial mail but maybe I wasn't clear enough: > > - in generate_icache_flush() and generate_verify_oop() we DO NOT > GENERATE any stub code. We don't use dynamically generated stubs on > ppc64 for flushing the icache or verifying oops but call C-functions > instead. So there's no need to generate post_dynamic_code_generated() > events for them and also no need for a StubCodeMark. > > - for generate_throw_exception() we dynamically generate a runtime > stub instead of an simple stub and for runtime stubs the JVMT dynamic > code event is already generated by RuntimeStub::new_runtime_stub() -> > CodeBlob::trace_new_stub() -> > JvmtiExport::post_dynamic_code_generated(). This is exactly the way > how it works on other CPU architectures. The usage of a StubCodeMark > in generate_throw_exception() was simply a "day one" bug in the ppc64 > port. > > > Thank you for the extra details! > I asked for that as my knowledge in this area is limited. > > The fix looks good to me. > I can be a sponsor for integration if needed. > But a Review is still required. > > > > - I haven't changed generate_handler_for_unsafe_access() so I don't > actually understand your concerns. > > > I accidentally copied a wrong name. Sorry. > I had to copy: generate_verify_oop(). > > Thanks, > Serguei > > generate_handler_for_unsafe_access() correctly contains a StubCodeMark > because it dynamically generates stub code - even if it is just the > output of a "not yet implemented" message. > > Regards, > Volker > > We also need someone from the compiler team to look at this. > I also included into the cc-list Oleg, who recently touched this area. > > Thanks, > Serguei > > > > On 7/14/14 11:24 AM, Volker Simonis wrote: > > Hi everybody, > > can somebody PLEASE review and sponsor this tiny, ppc64-only change. > > Thanks, > Volker > > > On Tue, Jul 8, 2014 at 5:45 PM, Daniel D. Daugherty > <daniel.daughe...@oracle.com> wrote: > > Adding the Serviceability Team since JVM/TI belongs to them. > > Dan > > > > On 7/8/14 9:41 AM, Volker Simonis wrote: > > Hi, > > could somebody please review and push the following small, PPC64-only > change to any of the hs team repositories: > > http://cr.openjdk.java.net/~simonis/webrevs/8049441/ > https://bugs.openjdk.java.net/browse/JDK-8049441 > > Background: > > For some stubs we actually do not really generate code on PPC64 but > instead we use a native C-function with inline-assembly. If the > generators of these stubs contain a StubCodeMark, they will trigger > JvmtiExport::post_dynamic_code_generated_internal events with a zero > length code size. These events may fool clients like Oprofile which > register for these events (thanks to Maynard Johnson who reported this > - see > http://mail.openjdk.java.net/pipermail/ppc-aix-port-dev/2014-June/002032.html). > > This change simply removes the StubCodeMark from > ICacheStubGenerator::generate_icache_flush() and generate_verify_oop() > because they don't generate assembly code. It also removes the > StubCodeMark from generate_throw_exception() because it doesn't really > generate a plain stub but a runtime stub for which the JVMT dynamic > code event is already generated by RuntimeStub::new_runtime_stub() -> > CodeBlob::trace_new_stub() -> > JvmtiExport::post_dynamic_code_generated(). > > Thank you and best regards, > Volker > > >
