RFR (L): 8024545: make develop and notproduct flag values available in product builds
Christian Thalinger
christian.thalinger at oracle.com
Wed Sep 25 14:17:13 PDT 2013
Oh well. It turned out that SunStudio cannot handle strings properly in the static table. So I chose another approach which actually uses less memory: for develop and notproduct flags define both a const variable and a static variable with CONST_ as a prefix. The address of the latter is then used in the flags table. No need for an additional _value field.
With that the binary size increase is:
linux_i486_compiler2: 24k
linux_i486_minimal1: 17k
I hope this is the last iteration:
http://cr.openjdk.java.net/~twisti/8024545/webrev/
The gist of this iteration is at the bottom of this page:
http://cr.openjdk.java.net/~twisti/8024545/webrev/src/share/vm/runtime/globals.hpp.udiff.html
On Sep 19, 2013, at 6:34 PM, Christian Thalinger <christian.thalinger at oracle.com> wrote:
> Thank you, again, Vladimir.
>
> On Sep 19, 2013, at 4:51 PM, Vladimir Kozlov <vladimir.kozlov at oracle.com> wrote:
>
>> Good.
>>
>> Vladimir
>>
>> On 9/19/13 4:08 PM, Christian Thalinger wrote:
>>> JPRT found some problems. Apparently something does #define C2 'E' on Solaris so I had to prefix all kind flags with KIND_:
>>>
>>> + // flag kind
>>> + KIND_PRODUCT = 1 << 4,
>>> + KIND_MANAGEABLE = 1 << 5,
>>> + KIND_DIAGNOSTIC = 1 << 6,
>>> + KIND_EXPERIMENTAL = 1 << 7,
>>> + KIND_NOT_PRODUCT = 1 << 8,
>>> + KIND_DEVELOP = 1 << 9,
>>> + KIND_PLATFORM_DEPENDENT = 1 << 10,
>>> + KIND_READ_WRITE = 1 << 11,
>>> + KIND_C1 = 1 << 12,
>>> + KIND_C2 = 1 << 13,
>>> + KIND_ARCH = 1 << 14,
>>> + KIND_SHARK = 1 << 15,
>>> + KIND_LP64_PRODUCT = 1 << 16,
>>> + KIND_COMMERCIAL = 1 << 17,
>>>
>>> Two other problems found by Visual Studio were a bool return:
>>>
>>> + bool Flag::get_bool() const {
>>> + if (is_constant_in_binary()) {
>>> + return _value != 0;
>>> + } else {
>>> + return *((bool*) _addr);
>>> + }
>>> + }
>>>
>>> and the size_t changes at the bottom of this file:
>>>
>>> http://cr.openjdk.java.net/~twisti/8024545/webrev/src/share/vm/runtime/globals.cpp.udiff.html
>>>
>>> Everything else (except the renames) are the same. I tried to create an incremental webrev but I failed. Sorry.
>>>
>>> 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.
>>>>> 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.
>>>>>>>>>>
>>>>>>>>
>>>>>>
>>>>
>>>
>
More information about the hotspot-dev
mailing list