[13] RFR(L) 8220623: [JVMCI] Update JVMCI to support JVMCI based Compiler compiled into shared library
Kim Barrett
kim.barrett at oracle.com
Fri Apr 5 23:56:15 UTC 2019
> On Apr 4, 2019, at 7:45 PM, Vladimir Kozlov <vladimir.kozlov at oracle.com> wrote:
>
> Thank you, Kim
>
> I moved HotSpotJVMCI methods which call JNIHandles::resolve() to jvmciJavaClasses.cpp file so that I don't need to include jniHandles.inline.hpp into .hpp file.
>
> I will update changes when I address your other comments.
Here are a few more comments.
I really want to better understand the MetadataHandle stuff, but I
don't think I will be able to seriously dig into it for a while. I'm
not sanguine about what seems to be yet another weak reference
mechanism.
------------------------------------------------------------------------------
src/hotspot/share/prims/jvmtiTagMap.cpp
3046 blk.set_kind(JVMTI_HEAP_REFERENCE_OTHER);
3047 Universe::oops_do(&blk);
3048
3049 #if INCLUDE_JVMCI
3050 blk.set_kind(JVMTI_HEAP_REFERENCE_OTHER);
3051 JVMCI::oops_do(&blk);
3052 if (blk.stopped()) {
3053 return false;
3054 }
3055 #endif
(New code starts with line 3049.)
There should probably be a blk.stopped() check after the call to
Universe::oops_do. This seems like a pre-existing bug, made more
apparent by the addition of the JVMCI code.
------------------------------------------------------------------------------
src/hotspot/share/classfile/classFileParser.cpp
5634 if (!is_internal()) {
5635 bool trace_class_loading = log_is_enabled(Info, class, load);
5636 #if INCLUDE_JVMCI
5637 bool trace_loading_cause = TraceClassLoadingCause != NULL &&
5638 (strcmp(TraceClassLoadingCause, "*") == 0 ||
5639 strstr(ik->external_name(), TraceClassLoadingCause) != NULL);
5640 trace_class_loading = trace_class_loading || trace_loading_cause;
5641 #endif
5642 if (trace_class_loading) {
5643 ResourceMark rm;
5644 const char* module_name = (module_entry->name() == NULL) ? UNNAMED_MODULE : module_entry->name()->as_C_string();
5645 ik->print_class_load_logging(_loader_data, module_name, _stream);
5646 #if INCLUDE_JVMCI
5647 if (trace_loading_cause) {
5648 JavaThread::current()->print_stack_on(tty);
5649 }
5650 #endif
This appears to be attempting to force a call to
print_class_load_logging if either log_is_enabled(Info, class, load)
is true or if TraceClassLoadingCause triggers it. But
print_class_load_logging does nothing if that logging is not enabled,
with the result that if it isn't enabled, but the tracing option
matches, we'll get a mysterious stack trace printed and nothing else.
I suspect that isn't the desired behavior.
------------------------------------------------------------------------------
src/hotspot/share/jvmci/jvmciRuntime.cpp
135 // The following instance variables are only used by the first block in a chain.
136 // Having two types of blocks complicates the code and the space overhead is negligible.
137 MetadataHandleBlock* _last; // Last block in use
138 intptr_t _free_list; // Handle free list
139 int _allocate_before_rebuild; // Number of blocks to allocate before rebuilding free list
There seems to be exactly one MetadataHandleBlock chain, in
_metadata_handles. So these instance variables could be static class
variables.
If there was more than one list, I think a better approach would have
a MetadataHandleBlockList class that had those members and a pointer
to the first block in the list. This code seems to have been pretty
much copy-paste-modified from JNIHandleBlock, which has somewhat
different requirements and usage pattern.
------------------------------------------------------------------------------
More information about the graal-dev
mailing list