RFR: 8194759: Support caching class mirror objects

[Ioi Lam]

Hi Jiangli,

The changes look good. I have a few style suggestions:

1.

Is the "classpath_index >= _app_paths_start_index" necessary in the 
following block? I think it's very rare that we would have signed 
classes in the bootappend JAR files, so removing this check would make 
the code simpler.

     // Ignore any App classes from signed JAR file during CDS archiving
     // dumping
     if (DumpSharedSpaces &&
SharedClassUtil::is_classpath_entry_signed(classpath_index) &&
         classpath_index >= _app_paths_start_index) {
       tty->print_cr("Preload Warning: Skipping %s from signed JAR",
                     context->class_name());
       return false;
     }


2.

For has_signer_and_not_archived -- I think it's still not clear that 
this function applies only during dumping.

How about always setting this flag (during run time and dump time), then 
it can be renamed simply as "has_signers()". This function would work 
both at run time and dump time (although there may not be a use case for 
this function during normal run time ...).


3.

I think you should move the THREAD_FIELDS_DO macro into javaClasses.hpp 
and use that to declare the fields for Thread as well. (Same for the 
other classes).

This would avoid code duplication. Also, this means you can't declare a 
new field without adding it to the THREAD_FIELDS_DO list.

#define THREAD_FIELDS_DO(macro) \
   macro(_name_offset,          k, vmSymbols::name_name(), 
string_signature, false); \
   macro(_group_offset,         k, vmSymbols::group_name(), 
threadgroup_signature, false); \
   macro(_contextClassLoader_offset, k, 
vmSymbols::contextClassLoader_name(), classloader_signature, false); \
   macro(_inheritedAccessControlContext_offset, k, 
vmSymbols::inheritedAccessControlContext_name(), 
accesscontrolcontext_signature, false); \
   macro(_priority_offset,      k, vmSymbols::priority_name(), 
int_signature, false); \
   macro(_daemon_offset,        k, vmSymbols::daemon_name(), 
bool_signature, false); \
   macro(_eetop_offset,         k, "eetop", long_signature, false); \
   macro(_stillborn_offset,     k, "stillborn", bool_signature, false); \
   macro(_stackSize_offset,     k, "stackSize", long_signature, false); \
   macro(_tid_offset,           k, "tid", long_signature, false); \
   macro(_thread_status_offset, k, "threadStatus", int_signature, false); \
   macro(_park_blocker_offset,  k, "parkBlocker", object_signature, 
false); \
   macro(_park_event_offset,    k, "nativeParkEventPointer", 
long_signature, false)

class java_lang_Thread : AllStatic {
  private:
   // Note that for this class the layout changed between JDK1.and JDK1.3,
   // so we compute the offsets at startup rather than hard-wiring them.
   static int _name_offset;
   static int _group_offset;
   static int _contextClassLoader_offset;
   static int _inheritedAccessControlContext_offset;
   static int _priority_offset;
   static int _eetop_offset;
   static int _daemon_offset;
   static int _stillborn_offset;
   static int _stackSize_offset;
   static int _tid_offset;
   static int _thread_status_offset;
   static int _park_blocker_offset;
   static int _park_event_offset ;

Thanks

- Ioi


On 2/22/18 10:24 PM, Jiangli Zhou wrote:
> Here is the new webrev with updates that address feedbacks from Misha, Ioi, Coleen and Thomas:
>
> http://cr.openjdk.java.net/~jiangli/8194759/webrev.01/ <http://cr.openjdk.java.net/~jiangli/8194759/webrev.01/>
>
> Incremental webrev:
> http://cr.openjdk.java.net/~jiangli/8194759/webrev.01.inc/ <http://cr.openjdk.java.net/~jiangli/8194759/webrev.01.inc/>
>
> Rerunning tiered testing ...
>
> Thanks,
> Jiangli
>