RFR 8041565: JMX ObjectName could be refactored to save memory

[Daniel Fuchs]

Hi Jaroslav,

I like this change, but it does introduce an incompatibility,
so it probably needs a CCC and some release notes.

For instance, this test passes with the current version of
ObjectName:

public class StringLengthTest {

     final static int smax = Short.MAX_VALUE;
     final static int smore = smax + 126;
     public static void main(String[] args) throws 
MalformedObjectNameException {
         char[] chars = new char[smore];
         Arrays.fill(chars, 0, smax, 'a');
         Arrays.fill(chars, smax, smore, 'b');

         System.out.println(new ObjectName(new String(chars), "type", 
"Test"));
     }

}

I'm not sure what it will do with your changes :-)

With that in mind I believe you should consider throwing
InternalError - or IllegalArgumentException - instead of
using 'assert' statements.

BTW have you run the JCK?

best regards,

-- daniel

On 13/04/15 17:07, Jaroslav Bachorik wrote:
> Hi Roger,
>
> On 13.4.2015 16:07, Roger Riggs wrote:
>> Hi Jaroslav,
>>
>> Minor comments:
>>
>> 1488+:  In forms like:
>>
>>   _pattern_flag &= (~PROPLIST_PATTERN & 0xff);"
>>
>> The &0xff seems unnecessary since the store is to a byte field.
>
> Fixed: http://cr.openjdk.java.net/~jbachorik/8041565/webrev.01
>
>>
>> 1644:  the ? and : operators should be surrounded by spaces.
>>
>> There are other style issues, such as then statements on the same line
>> as the 'if' but that
>> may be beyond the scope of this change.
>
> I know but these style issue have been around since the file was first
> committed. I didn't address them because it didn't feel right to mix
> style changes with the actual functionality.
>
> Cheers,
>
> -JB-
>
>>
>> Otherwise looks fine  (as a 9 reviewer, but not specifically a
>> serviceability reviewer).
>>
>> Thanks, Roger
>>
>>
>> On 4/13/2015 5:43 AM, Jaroslav Bachorik wrote:
>>> Please, review the following change
>>>
>>> Issue : https://bugs.openjdk.java.net/browse/JDK-8041565
>>> Webrev: http://cr.openjdk.java.net/~jbachorik/8041565/webrev.00
>>>
>>> In situations when there are 10s of thousands ObjectNname instances
>>> around (enterprise setups etc.) the 3 separate internal boolean fields
>>> can lead to a noticeable memory waste. Adding insult to the injury,
>>> with the current field layout it is necessary to align the instances
>>> by 4 bytes.
>>>
>>> When using JOL (http://openjdk.java.net/projects/code-tools/jol/) to
>>> inspect the object layout we can see this:
>>>
>>> Before optimization (JDK8u40):
>>> ---
>>> javax.management.ObjectName object internals:
>>> OFFSET SIZE TYPE DESCRIPTION VALUE
>>>       0 12 (object header)| N/A
>>>      12 4 int ObjectName._domain_length N/A
>>>      16 1 boolean ObjectName._domain_pattern N/A
>>>      17 1 boolean ObjectName._property_list_pattern N/A
>>>      18 1 boolean ObjectName._property_value_pattern N/A
>>>      19 1 (alignment/padding gap) N/A
>>>      20 4 String ObjectName._canonicalName N/A
>>>      24 4 Property[] ObjectName._kp_array N/A
>>>      28 4 Property[] ObjectName._ca_array N/A
>>>      32 4 Map ObjectName._propertyList N/A
>>>      36 4 (loss due to the next object alignment)
>>> Instance size: 40 bytes (estimated, the sample instance is not
>>> available)
>>> Space losses: 1 bytes internal + 4 bytes external = 5 bytes total
>>> {noformat}
>>>
>>> After optimization (JDK9 internal build):
>>> ---
>>>
>>> javax.management.ObjectName object internals:
>>>  OFFSET SIZE TYPE DESCRIPTION VALUE
>>>       0 12 (object header) N/A
>>>      12 2 short ObjectName._domain_length N/A
>>>      14 1 byte ObjectName._pattern_flag N/A
>>>      15 1 (alignment/padding gap) N/A
>>>      16 4 String ObjectName._canonicalName N/A
>>>      20 4 Property[] ObjectName._kp_array N/A
>>>      24 4 Property[] ObjectName._ca_array N/A
>>>      28 4 Map ObjectName._propertyList N/A
>>> Instance size: 32 bytes (estimated, the sample instance is not
>>> available)
>>> Space losses: 1 bytes internal + 0 bytes external = 1 bytes total
>>>
>>>
>>> After optimization we can save 8 bytes per instance which can
>>> translate to very interesting numbers on large installations.
>>>
>>>
>>> To achieve this the domain name length is set to be *short* instead of
>>> *int* and the three booleans kept for the performance purposes are
>>> encoded into one byte value (as proposed by the reporter,
>>> Jean-Francois Denise).
>>>
>>> All the regression and JCK tests are passing after this change.
>>>
>>>
>>> Thanks,
>>>
>>> -JB-
>>
>