Re: RFR(M): 8201375: Add the AllowArchivingWithJavaAgent diagnostic vm option to allow the use of the -javaagent option during CDS dumping

Thu, 15 Nov 2018 22:53:11 -0800

On 11/15/18 22:39, [email protected] wrote:
Hi Calvin,

It seems the option name AllowArchivingWithJavaAgent is not right.
In fact, it is about JVMTI (native) agents:

  // Create agents for -agentlib:  -agentpath:  and converted -Xrun
  // Invokes Agent_OnLoad
  // Called very early -- before JavaThreads exist
  void Threads::create_vm_init_agents() {
+   if (DumpSharedSpaces && !AllowArchivingWithJavaAgent) {
+     vm_exit_during_cds_dumping(
+         "Must enable AllowArchivingWithJavaAgent in order to run Java agent during CDS dumping");
+   }
    extern struct JavaVM_ main_vm;
    AgentLibrary* agent;
  
    JvmtiExport::enter_onload_phase();
  
    for (agent = Arguments::agents(); agent != NULL; agent = agent->next()) {
+     // CDS dumping does not support native JVMTI agent
+     if (DumpSharedSpaces && !agent->is_instrument_lib()) {
+       vm_exit_during_cds_dumping("CDS dumping does not support native JVMTI agent, name", agent->name());
+     }
+ 
      OnLoadEntry_t  _on_load_entry_ = lookup_agent_on_load(agent);
The flags -agentlib and -agentpath are to run JVMTI (native) agents,
and Agent_OnLoad is an entrypoint in native agent library.
So, I'm thinking if it'd make sense to rename the option to something like below:
  AllowArchivingWithJVMTIAgent or
  AllowArchivingWithNativeAgent or
  AllowArchivingWithAgent


http://cr.openjdk.java.net/%7Eccheung/8201375/webrev.00/src/hotspot/share/memory/filemap.cpp.frames.html
 212   _allow_archiving_with_java_agent = AllowArchivingWithJavaAgent;
 ...

1350   if (_allow_archiving_with_java_agent && !AllowArchivingWithJavaAgent) {
1351     FileMapInfo::fail_continue("The setting of the AllowArchivingWithJavaAgent is different "
1352                                "from the setting in the shared archive.");
1353     return false;
1354   }

The _allow_archiving_with_java_agent is initialized with the AllowArchivingWithJavaAgent at line 212.
It is not clear from scratch how these two values can be different at line 1350.
At least, some comment explaining it is needed.
To make it more clear...
The error message does not help much for people who are not used to create and use archives.
Does it mean that the field _allow_archiving_with_java_agent is set only at the dumping phase,
but the AllowArchivingWithJavaAgent has to be always passed?


Thanks,
Serguei
Thanks,
Serguei


On 11/15/18 10:56, Jiangli Zhou wrote:
+1

Adding serviceability-dev mailing list. It would be good to have this reviewed by the JVMTI experts also to make sure all cases are covered.

Thanks,

Jiangli


On 11/15/18 9:29 AM, Ioi Lam wrote:
Hi Calvin,

The changes look good.

Thanks

- Ioi

On 11/14/18 4:40 PM, Calvin Cheung wrote:
bug: https://bugs.openjdk.java.net/browse/JDK-8201375

webrev: http://cr.openjdk.java.net/~ccheung/8201375/webrev.00/

This change is for adding a diagnostic vm option (AllowArchivingWithJavaAgent) to allow the -javaagent in the command line during CDS dumping. Please refer to the corresponding CSR (https://bugs.openjdk.java.net/browse/JDK-8213813) for details.

Testing: ran hs-tier{1,2,3} tests through mach5.

thanks,
Calvin



Reply via email to