RFR (L): 8024545: make develop and notproduct flag values available in product builds

[Christian Thalinger]

On Sep 12, 2013, at 6:00 PM, Christian Thalinger <christian.thalinger at oracle.com> wrote:

> 
> On Sep 12, 2013, at 5:24 PM, Vladimir Kozlov <vladimir.kozlov at oracle.com> wrote:
> 
>> On 9/12/13 5:11 PM, Christian Thalinger wrote:
>>> 
>>> On Sep 12, 2013, at 4:30 PM, Vladimir Kozlov <vladimir.kozlov at oracle.com> wrote:
>>> 
>>>> Christian,
>>>> 
>>>> Can you put _flags first to avoid padding between _addr and _value in 32-bit VM?
>>> 
>>> Yes, good point.
>>> 
>>>> 
>>>> It would be nice enumerate flag's type and use int instead of strings:
>>>> 
>>>> enum {
>>>> bool_flag = 1,
>>>> intx_flag = 2,
>>>> 
>>>> + #define RUNTIME_PRODUCT_FLAG_STRUCT(     type, name, value, doc) { type##_flag, XSTR(name), &name,              VALUE(value), NOT_PRODUCT_ARG(doc) Flag::Flags(Flag::DEFAULT | Flag::KIND_PRODUCT) },
>>> 
>>> I was thinking of putting the type into the flags, too.  That would save another pointer word.  Should I give it a shot in this change or a separate one?
>> 
>> Do it in separate changes after you push this.

FTR:

8024803: move Flag::_type to Flag::_flags

-- Chris

>> And if you remove _type field later you don't need to move _flags now.
>> So you can push your current changes as it is. They are good.
> 
> Thank you, Vladimir.  -- Chris
> 
>> 
>> Thanks,
>> Vladimir
>> 
>>> 
>>> -- Chris
>>> 
>>>> 
>>>> Thanks,
>>>> Vladimir
>>>> 
>>>> On 9/12/13 9:58 AM, Christian Thalinger wrote:
>>>>> 
>>>>> On Sep 11, 2013, at 5:34 PM, David Holmes <david.holmes at oracle.com> wrote:
>>>>> 
>>>>>> Hi Chris,
>>>>>> 
>>>>>> There seems to be an awful lot "infrastructure" work needed to make something that sounds simple happen. :( All the changes to the scoping and the set/get methods tends to obscure the core change. My only suggestion here is that the "set" methods could perhaps factor this:
>>>>>> 
>>>>>> +   if (is_constant_in_binary()) {
>>>>>> +     fatal(err_msg("flag is constant: %s", _name));
>>>>>> 
>>>>>> into a check_writable() method so that it isn't duplicated so much.
>>>>> 
>>>>> Good point.  I made that change:
>>>>> 
>>>>> http://cr.openjdk.java.net/~twisti/8024545/webrev/src/share/vm/runtime/globals.cpp.udiff.html
>>>>> 
>>>>>> 
>>>>>> I also wonder whether a ConstFlag sub/superclass would simplify this at all?
>>>>> 
>>>>> Maybe it would but then we need more infrastructure to read the additional entries.  Might be a wash.  SA only knows about offsets in structs and the flags array is statically defined.
>>>>> 
>>>>>> 
>>>>>> That aside I'm curious why the minimal VM size change is only 22K when client is 32K?
>>>>> 
>>>>> The 32-bit product build also contains the server compiler.
>>>>> 
>>>>> -- Chris
>>>>> 
>>>>>> 
>>>>>> Thanks,
>>>>>> David
>>>>>> 
>>>>>> On 11/09/2013 10:56 AM, Christian Thalinger wrote:
>>>>>>> http://cr.openjdk.java.net/~twisti/8024545/webrev/
>>>>>>> 
>>>>>>> 8024545: make develop and notproduct flag values available in product builds
>>>>>>> Reviewed-by:
>>>>>>> 
>>>>>>> Right now the internal flag table only contains flags which are defined in a product build. This does not include develop and notproduct flags. Sometimes it is useful to have access to these values for post-mortem core file analysis or to read these values for compiler settings for a Java-based compiler.
>>>>>>> 
>>>>>>> This change enables develop and notproduct flag values to be read by the serviceability agent. The binary size is increased by 42k for a 64-bit product build and by 32k for a 32-bit product build.
>>>>>>> 
>>>>>>> Before:
>>>>>>> 
>>>>>>> $ java -cp /java/re/jdk/8/latest/binaries/linux-x64/lib/sa-jdi.jar sun.jvm.hotspot.CLHSDB 9399
>>>>>>> Attaching to process 9399, please wait...
>>>>>>> hsdb> flags -nd
>>>>>>> InitialHeapSize = 495006528 5
>>>>>>> MaxHeapSize = 7920943104 5
>>>>>>> UseCompressedKlassPointers = true 5
>>>>>>> UseCompressedOops = true 5
>>>>>>> UseParallelGC = true 5
>>>>>>> hsdb> flags InlineMathNatives
>>>>>>> Couldn't find flag: InlineMathNatives
>>>>>>> 
>>>>>>> After:
>>>>>>> 
>>>>>>> $ java -cp $JAVA_HOME/lib/sa-jdi.jar sun.jvm.hotspot.CLHSDB 3726
>>>>>>> Attaching to process 3726, please wait...
>>>>>>> hsdb> flags -nd
>>>>>>> InitialHeapSize = 495006528 5
>>>>>>> MaxHeapSize = 7920943104 5
>>>>>>> UseCompressedKlassPointers = true 5
>>>>>>> UseCompressedOops = true 5
>>>>>>> UseParallelGC = true 5
>>>>>>> hsdb> flags InlineMathNatives
>>>>>>> InlineMathNatives = true 0
>>>>>>> 
>>>>>>> This patch has one behavioral difference; when printing flags with e.g. PrintFlagsFinal in a debug build it prints "develop" for develop flags:
>>>>>>> 
>>>>>>>    uintx AdaptiveSizePolicyGCTimeLimitThreshold    = 5               {develop}
>>>>>>> 
>>>>>>> The output for product builds is unchanged.
>>>>>>> 
>>>>> 
>>> 
>