ARM: Support for JVMTI notifications from JIT compiler on code generation

[Andrew Haley]

On 04/25/2012 01:49 PM, Andrew Dinn wrote:
> This is a patch against
> http://icedtea.classpath.org/hg/release/icedtea7-forest-2.1/hotspot
> 
> It modifies the ARM JIT to post JVMTI events whenever native code is
> generated from Java method bytecode, providing details of the class
> andmethod name, the generated code address range and a mapping from
> address subranges to bytecode indices. That's enough to allow oprofile
> to provide line by line profiling information.
> 
> This has been tested with the oprofile Java agent and the resulting
> behaviour appears to be compatible with behaviour on x86. However, this
> is not a full test as oprofile is not fully working in the latest
> available ARM kernel. This means that the data generated by the agent in
> response to the JVMTI events has not yet been properly consumed and
> processed.
> 
> I have asked Andrew Haley to review this patch.

But if anyone else wants to join in...

> diff -r a5d04cb60a5c -r fada6a7d5e89 src/cpu/zero/vm/thumb2.cpp
> --- a/src/cpu/zero/vm/thumb2.cpp	Wed Apr 11 09:24:03 2012 -0400
> +++ b/src/cpu/zero/vm/thumb2.cpp	Wed Apr 25 13:43:33 2012 +0100
> @@ -440,6 +440,12 @@
>  unsigned stack[1000];
>  unsigned r_local[1000];
>  
> +// jvmti needs to map start address of generated code for a bytecode
> +// to corrsponding bytecode index so agents can correlate code address
> +// ranges with bci and thence line number
> +jvmtiAddrLocationMap address_bci_map[THUMB2_MAX_BYTECODE_SIZE];

I presume that there need only be one of these because it is only
accessed under lock.  However, it's 160kbytes in size.  It
might be a better idea to allocate the space at runtime: not every
VM runs the JIT.

>    jinfo_str.code_base = code_base;
> @@ -7580,6 +7597,27 @@
>    if (compiled_offset == 0) return 0;
>    thumb_entry.compiled_entrypoint = slow_entry + compiled_offset;
>    thumb_entry.osr_entry = (unsigned)cmethod->osr_entry | TBIT;
> +  {
> +    // we need to notify a Jvmti compiled_method_load event
> +
> +    // notify the whole generated code region for this Java method
> +    // from slow_entry through to the end of the osr table. some
> +    // of it is data not code but that's not a problem.

I have a personal preference for sentences in comments.


> +
> +    const void *gen_code_start = (const void *)(slow_entry ^ TBIT);
> +    unsigned gen_code_size = codebuf_str.idx * 2;
> +
> +    // address_bci_map translates start addresses for generated code
> +    // sections to bytecode indices
> +
> +    // the final compile_info argument is supposed to contain
> +    // information about inlined code. we can supply NULL for now -
> +    // oprofile doesn't use it anyway
> +
> +    void *compile_info = NULL;
> +
> +    JvmtiExport::post_compiled_method_load(method, gen_code_size, gen_code_start, address_bci_map_length, address_bci_map, NULL);

Please avoid lines so long that they wrap.

> +  }
>    return *(unsigned long long *)&thumb_entry;
>  }
>  
> diff -r a5d04cb60a5c -r fada6a7d5e89 src/share/vm/prims/jvmtiExport.cpp
> --- a/src/share/vm/prims/jvmtiExport.cpp	Wed Apr 11 09:24:03 2012 -0400
> +++ b/src/share/vm/prims/jvmtiExport.cpp	Wed Apr 25 13:43:33 2012 +0100
> @@ -1776,6 +1776,48 @@
>    }
>  }
>  
> +#ifdef __arm__
> +
> +// special compiled_method_load notify API for thumb2 compiler
> +
> +void JvmtiExport::post_compiled_method_load(const methodOop method, const jint length,
> +                                            const void *code_begin, const jint map_length,
> +                                            const jvmtiAddrLocationMap* map,
> +					    const void *compile_info)
> +{
> +  JavaThread* thread = JavaThread::current();
> +  jmethodID methodId = method->jmethod_id();
> +
> +  EVT_TRIG_TRACE(JVMTI_EVENT_COMPILED_METHOD_LOAD,
> +                 ("JVMTI [%s] method compile load event triggered (by thumb2_compile)",
> +                 JvmtiTrace::safe_get_thread_name(thread)));
> +
> +  JvmtiEnvIterator it;
> +  for (JvmtiEnv* env = it.first(); env != NULL; env = it.next(env)) {
> +    if (env->is_enabled(JVMTI_EVENT_COMPILED_METHOD_LOAD)) {
> +
> +      EVT_TRACE(JVMTI_EVENT_COMPILED_METHOD_LOAD,
> +                ("JVMTI [%s] class compile method load event sent %s.%s   (by thumb2_compile)",
> +                JvmtiTrace::safe_get_thread_name(thread),
> +                method->klass_name()->as_C_string(),
> +		 method->name()->as_C_string()));
> +
> +      // I think we need this to add a resource mark and handle for
> +      // the thread and record a vm to native (agent lib) transition
> +
> +      JvmtiJavaThreadEventTransition jet(thread);
> +      jvmtiEventCompiledMethodLoad callback = env->callbacks()->CompiledMethodLoad;
> +
> +      if (callback != NULL) {
> +        (*callback)(env->jvmti_external(), methodId,
> +                    length, code_begin, map_length,
> +                    map, compile_info);
> +      }
> +    }
> +  }
> +}
> +
> +#endif // __arm__

This is a bit unfortunate.  I presume we have to do it because JvmtiExport
doesn't give us what we need.

Andrew.