RFR(XS): 8049441: PPC64: Don't use StubCodeMarks for zero-length stubs

[Volker Simonis]

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.spitsyn at oracle.com
<serguei.spitsyn at oracle.com> wrote:
> On 7/15/14 1:48 AM, Volker Simonis wrote:
>
> On Mon, Jul 14, 2014 at 10:35 PM, serguei.spitsyn at oracle.com
> <serguei.spitsyn at 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.daugherty at 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
>
>
>