'AllowArchivingWithAgent' is also one of the earlier suggested naming (I
voted for it :)). I think it servers the purpose better. We mainly want
a diagnosis flag that allows us to use a JVMTI agent at CDS dump time
for testing purpose only. Even for the 'allowed' usage, it may not have
a complete support for all cases (so a diagnosis flag is needed to turn
on the mode).
Thanks,
Jiangli
On 11/16/18 6:08 AM, Daniel D. Daugherty wrote:
> The flags -agentlib and -agentpath are to run JVMTI (native) agents,
> and Agent_OnLoad is an entrypoint in native agent library.
The '-agentlib' and '-agentpath' options are used to load and execute
a native library via the Agent_OnLoad() entry point. However, the
agent does not have to use JVM/TI (it could be pure JNI):
$ java -help
-agentlib:<libname>[=<options>]
load native agent library <libname>, e.g.
-agentlib:hprof
see also, -agentlib:jdwp=help and -agentlib:hprof=help
-agentpath:<pathname>[=<options>]
load native agent library by full pathname
The '-javaagent' option is used to load and execute a Java agent:
-javaagent:<jarpath>[=<options>]
load Java programming language agent, see
java.lang.instrument
so I agree that the option is misnamed (a bit), but the reasons
needed to be clarified.
Dan
On 11/16/18 1:39 AM, serguei.spit...@oracle.com 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.
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