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

[Jiangli Zhou]

'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.spitsyn at 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
>>>
>>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/serviceability-dev/attachments/20181116/d45ac50b/attachment.html>