Review Request: 8029921: CDS should allow user specify their own classlist

David Holmes david.holmes at oracle.com
Wed Jan 15 19:41:49 PST 2014 

On 16/01/2014 3:26 AM, Coleen Phillimore wrote:
>
> 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.

I beg to differ. What we are proposing here is a change to the user 
interface to CDS, which is already defined using the supported -Xshare 
family of options. All the flags you mention above are simple CDS 
implementation related settings and quite rightly they belong as -XX 
flags as they pertain to the hotspot implementation of CDS.

>> 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.

I wasn't aware of the CDS-on-server work specifically but with App CDS 
we are looking at numerous changes in this area both in regards to the 
classlists and the archives themselves. What we are discussing here is a 
first step, for Embedded 8u6 primarily) that allows the default 
classlist to be replaced to suit the application.

>> -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?

That would be useful but is not in scope for this small enhancement.

David
-----

> 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