Review Request: 8029921: CDS should allow user specify their own classlist
Coleen Phillimore
coleen.phillimore at oracle.com
Wed Jan 15 09:26:05 PST 2014
Hi, Some comments below.
On 1/15/2014 10:22 AM, Bob Vandette wrote:
> On Jan 15, 2014, at 1:16 AM, David Holmes <David.Holmes at oracle.com> wrote:
>
>> Hi Jiangli,
>>
>> On 15/01/2014 3:19 AM, Jiangli Zhou wrote:
>>> Hi David,
>>>
>>> Thanks for the suggestion. How does it work with the existing
>>> -Xshare:dump/auto/on option? Is it possible to combine multiple
>>> sub-options, so it would be something like -Xshare:dump,classlist=XXXX?
>> Right - the classlist is only associated with dumping so some modification to -Xshare:dump would be the way to do it. Exactly how depends on what we think we may want to do in the future. For example do we want to allow control over the name of the archive? If so we might want something like:
>>
>> -Xshare:dump:in=xxx,out=yyy
>>
>> though I'm not sure of the best syntax/grammar to use for these kind of compound options.
>>
>> At this stage we only want to define the class list file name so we don't need to generalize too far, but we also want to choose a syntax that doesn't preclude potential future changes.
>>
>> Comments from runtime folk (Hi Coleen!) would be appreciated. :)
> Do we want the ability to select different archives at startup that have been dumped with different class lists?
We have this already in diagnostic mode, which is used for testing:
diagnostic(ccstr, SharedArchiveFile,
NULL, \
"Override the default location of the CDS archive
file") \
I don't know how these composite options work or how many different
things you can add on the end or if it's even any more intuitive than
using similar names. We seem to use similar names more frequently in
order to group options. The other CDS options are that way:
product(bool, PrintSharedSpaces,
false, \
"Print usage of shared
spaces") \
\
product(uintx, SharedReadWriteSize, NOT_LP64(12*M)
LP64_ONLY(16*M), \
"Size of read-write space for metadata (in
bytes)") \
\
product(uintx, SharedReadOnlySize, NOT_LP64(12*M)
LP64_ONLY(16*M), \
"Size of read-only space for metadata (in
bytes)") \
\
product(uintx, SharedMiscDataSize, NOT_LP64(2*M)
LP64_ONLY(4*M), \
"Size of the shared miscellaneous data area (in
bytes)") \
\
product(uintx, SharedMiscCodeSize, 120*K, \
"Size of the shared miscellaneous code area (in
bytes)") \
\
product(uintx, SharedBaseAddress,
LP64_ONLY(32*G) \
NOT_LP64(LINUX_ONLY(2*G)
NOT_LINUX(0)), \
"Address to allocate shared memory region for class
data") \
I don't think Jiangli's option about the new classlist should be
different. Or we should change all of them, which seems like a lot of
extra work. This syntax seems a bit awful too but that's just my
feeling. I am getting tired of all these different command syntaxes.
>
> If so, then we'd need a way to specify the archive for -Xshare:on, ie -Xshare:on,archive=xxxx
>
> -Xshare:dump,list=xxx,archive=yyy
> -Xshare:on,archive=yyy
>
> Otherwise if we don't want to support multiple archives, this option would be enough.
We generate one archive and put it in the <cpu>/client jre directory.
We're going to need another for the <cpu>/server directory when we
officially support CDS on server. (It works now but we're not
advertising it because it hasn't gotten enough testing yet). In order
to have server CDS archive though, we do need to specify a different
classlist because the classlist is saved in jre/lib is used by default
for -Xshare:dump and has been generated for client applications.
> -Xshare:dump,list=xxx
>
> Can we make the classlist easy to generate but either supporting the -verbose:class output format or
> by adding a new -verbose:classlist option, please?
There is a way to do this but it's not easy and it's in the JDK
somewhere. Harold knows.
Thanks,
Coleen
>
>
> Bob.
>
>
>> Thanks,
>> David
>>
>>> Thanks,
>>> Jiangli
>>>
>>> On 01/13/2014 06:29 PM, David Holmes wrote:
>>>> On further thought I think this should be handled as part of the
>>>> -Xshare option eg -Xshare:classlist=XXXX, as this is intended to be a
>>>> supported feature of CDS.
>>>>
>>>> That of course requires CCC approval processes to be followed.
>>>>
>>>> David
>>>>
>>>> On 8/01/2014 3:33 PM, David Holmes wrote:
>>>>> On 8/01/2014 3:28 PM, David Holmes wrote:
>>>>>> Looks fine - thanks.
>>>>> Oops missed the follow-up. The further rename is fine too.
>>>>>
>>>>> David
>>>>>
>>>>>> David
>>>>>>
>>>>>> On 8/01/2014 3:10 AM, Jiangli Zhou wrote:
>>>>>>> Hi David,
>>>>>>>
>>>>>>> Here is the updated webrev:
>>>>>>> http://cr.openjdk.java.net/~jiangli/8029921/webrev.01/.
>>>>>>>
>>>>>>> Thanks,
>>>>>>> Jiangli
>>>>>>>
>>>>>>> On 01/06/2014 05:59 PM, Jiangli Zhou wrote:
>>>>>>>> Hi David,
>>>>>>>>
>>>>>>>> Thanks for the review. Will incorporate your suggestions.
>>>>>>>>
>>>>>>>> Thanks,
>>>>>>>> Jiangli
>>>>>>>>
>>>>>>>> On 01/06/2014 05:56 PM, David Holmes wrote:
>>>>>>>>> Hi Jiangli,
>>>>>>>>>
>>>>>>>>> On 7/01/2014 5:48 AM, Jiangli Zhou wrote:
>>>>>>>>>> Hi,
>>>>>>>>>>
>>>>>>>>>> Please review following change that allows alternative CDS
>>>>>>>>>> classlist
>>>>>>>>>> for
>>>>>>>>>> archiving by specifying -XX:ClassList=<path> from the command-line:
>>>>>>>>>>
>>>>>>>>>> webrev: http://cr.openjdk.java.net/~jiangli/8029921/webrev.00/
>>>>>>>>>> bug: https://bugs.openjdk.java.net/browse/JDK-8029921
>>>>>>>>> I think ClassListFile would be a better name to make it clear it is
>>>>>>>>> in fact a file name.
>>>>>>>>>
>>>>>>>>> In metaSpaceShared.cpp this comment block:
>>>>>>>>>
>>>>>>>>> 653 // Construct the path to the class list (in jre/lib)
>>>>>>>>> 654 // Walk up two directories from the location of the VM and
>>>>>>>>> 655 // optionally tack on "lib" (depending on platform)
>>>>>>>>>
>>>>>>>>> should moved into the if block after line #657.
>>>>>>>>>
>>>>>>>>> Thanks,
>>>>>>>>> David
>>>>>>>>>
>>>>>>>>>> Thanks,
>>>>>>>>>> Jiangli
More information about the hotspot-runtime-dev
mailing list