RFR 8041565: JMX ObjectName could be refactored to save memory

[Jaroslav Bachorik]

On 14.4.2015 14:56, Daniel Fuchs wrote:
> 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 :-)

It will fail with assert (if enabled). Or truncate the domain name, I 
suppose.

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

This would probably be better.

>
> BTW have you run the JCK?

Yes. All passed. I don't think JCK is testing for unrealistic values :) 
I don't see how one could come up with a domain name 32767 characters long.

-JB-

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