[prev in list] [next in list] [prev in thread] [next in thread]
List: openjdk-serviceability-dev
Subject: Re: RFR 8041565: JMX ObjectName could be refactored to save memory
From: Eamonn McManus <eamonn () mcmanus ! net>
Date: 2015-08-13 19:31:15
Message-ID: CACBEn44kmQrj_cJc-qkS_GsJ7K7Dx_PSMHOQdKw7q09bgXxdBQ () mail ! gmail ! com
[Download RAW message or body]
Looks fine to me (emcmanus).
On Aug 13, 2015 11:13 AM, "Jaroslav Bachorik" <jaroslav.bachorik@oracle.com>
wrote:
> On 13.8.2015 02:53, Daniel Fuchs wrote:
>
>> Hi Jaroslav,
>>
>> On 12/08/15 18:05, Jaroslav Bachorik wrote:
>>
>>> On 5.8.2015 08:04, Eamonn McManus wrote:
>>>
>>>> That makes me sad. The limit is clearly a detail of this particular
>>>> implementation and should not be enshrined in the spec.
>>>> Éamonn
>>>>
>>>
>>> I re-requested CCC for not mentioning the exact limit in the docs and it
>>> has been approved (thanks to Joe for quick turnaround).
>>>
>>> The update webrev:
>>> http://cr.openjdk.java.net/~jbachorik/8041565/webrev.04
>>>
>>
>> Why are index and in_index now declared as short? Is that a left over
>> from the original webrev?
>>
>> 445 short index = 0;
>> 501 short in_index;
>>
>
> Thanks for catching this!
>
> Updated: http://cr.openjdk.java.net/~jbachorik/8041565/webrev.05
>
> I'd better wait for Eamonn before proceeding with integration.
>
> -JB-
>
>
>> should these be reverted to 'int' ?
>>
>> Otherwise - this looks good...
>>
>> -- daniel
>>
>>
>>
>>> -JB-
>>>
>>>
>>>>
>>>> 2015-08-05 8:02 GMT-07:00 Jaroslav Bachorik
>>>> <jaroslav.bachorik@oracle.com>:
>>>>
>>>>> On 5.8.2015 16:53, Eamonn McManus wrote:
>>>>>
>>>>>>
>>>>>> I would remove the spec changes about the limit on the domain length,
>>>>>> which are a property of this particular implementation. It's perfectly
>>>>>> reasonable to blow up if the domain length is > 536,870,911, but
>>>>>> there's no reason for it to be in the spec.
>>>>>>
>>>>>
>>>>>
>>>>> Well, CCC board had a different opinion. That's why this limit is
>>>>> documented
>>>>> in the spec.
>>>>>
>>>>> -JB-
>>>>>
>>>>>
>>>>> Éamonn
>>>>>>
>>>>>>
>>>>>> 2015-08-05 4:48 GMT-07:00 Jaroslav Bachorik
>>>>>> <jaroslav.bachorik@oracle.com>:
>>>>>>
>>>>>>>
>>>>>>> Eamonn, Daniel,
>>>>>>>
>>>>>>> thanks for the comments.
>>>>>>>
>>>>>>> I've updated the webrev to address them. Also, I've added a test to
>>>>>>> exercise
>>>>>>> the boolean flag en-/decoding in ObjectName.
>>>>>>>
>>>>>>> http://cr.openjdk.java.net/~jbachorik/8041565/webrev.03
>>>>>>>
>>>>>>>
>>>>>>> Cheers,
>>>>>>>
>>>>>>> -JB-
>>>>>>>
>>>>>>>
>>>>>>> On 4.8.2015 23:02, Daniel Fuchs wrote:
>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> Hi Jaroslav,
>>>>>>>>
>>>>>>>> 379 * This field encodes _domain_pattern,
>>>>>>>> _property_list_pattern
>>>>>>>> and
>>>>>>>> 380 * _property_value_pattern booleans.
>>>>>>>>
>>>>>>>> If I'm not mistaken it also encodes the domain length.
>>>>>>>>
>>>>>>>>
>>>>>>>> 1072 if ((l & FLAG_MASK) > 0 ) {
>>>>>>>>
>>>>>>>> Although it is not expected that 'l' could be negative, it might be
>>>>>>>> better to write this test as:
>>>>>>>>
>>>>>>>> if ((l & FLAG_MASK) != 0 ) {
>>>>>>>>
>>>>>>>> (+ I agree with Éamonn that l is not a great parameter name -
>>>>>>>> nice to
>>>>>>>> see you back Éamonn :-)) best regards, -- daniel On 8/4/15 4:20 PM,
>>>>>>>> Jaroslav Bachorik wrote:
>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Hi, reviving this review. On 14.4.2015 16:58, Jaroslav Bachorik
>>>>>>>>> wrote:
>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> 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.
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> The proposed change has passed the CCC review. In case the domain
>>>>>>>>> name
>>>>>>>>> is longer than Integer.MAX_VALUE/4 a MalformedObjectNameException
>>>>>>>>> will
>>>>>>>>> be thrown. All the JMX tests and JCK are still passing.
>>>>>>>>> http://cr.openjdk.java.net/~jbachorik/8041565/webrev.02 -JB-
>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> -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-
>>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>
>>>
>>
>
[Attachment #3 (text/html)]
<p dir="ltr">Looks fine to me (emcmanus).</p>
<div class="gmail_quote">On Aug 13, 2015 11:13 AM, "Jaroslav Bachorik" \
<<a href="mailto:jaroslav.bachorik@oracle.com">jaroslav.bachorik@oracle.com</a>> \
wrote:<br type="attribution"><blockquote class="gmail_quote" style="margin:0 0 0 \
.8ex;border-left:1px #ccc solid;padding-left:1ex">On 13.8.2015 02:53, Daniel Fuchs \
wrote:<br> <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px \
#ccc solid;padding-left:1ex"> Hi Jaroslav,<br>
<br>
On 12/08/15 18:05, Jaroslav Bachorik wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc \
solid;padding-left:1ex"> On 5.8.2015 08:04, Eamonn McManus wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc \
solid;padding-left:1ex"> That makes me sad. The limit is clearly a detail of this \
particular<br> implementation and should not be enshrined in the spec.<br>
Éamonn<br>
</blockquote>
<br>
I re-requested CCC for not mentioning the exact limit in the docs and it<br>
has been approved (thanks to Joe for quick turnaround).<br>
<br>
The update webrev:<br>
<a href="http://cr.openjdk.java.net/~jbachorik/8041565/webrev.04" rel="noreferrer" \
target="_blank">http://cr.openjdk.java.net/~jbachorik/8041565/webrev.04</a><br> \
</blockquote> <br>
Why are index and in_index now declared as short? Is that a left over<br>
from the original webrev?<br>
<br>
445 short index = 0;<br>
501 short in_index;<br>
</blockquote>
<br>
Thanks for catching this!<br>
<br>
Updated: <a href="http://cr.openjdk.java.net/~jbachorik/8041565/webrev.05" \
rel="noreferrer" target="_blank">http://cr.openjdk.java.net/~jbachorik/8041565/webrev.05</a><br>
<br>
I'd better wait for Eamonn before proceeding with integration.<br>
<br>
-JB-<br>
<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc \
solid;padding-left:1ex"> <br>
should these be reverted to 'int' ?<br>
<br>
Otherwise - this looks good...<br>
<br>
-- daniel<br>
<br>
<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc \
solid;padding-left:1ex"> <br>
-JB-<br>
<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc \
solid;padding-left:1ex"> <br>
<br>
2015-08-05 8:02 GMT-07:00 Jaroslav Bachorik<br>
<<a href="mailto:jaroslav.bachorik@oracle.com" \
target="_blank">jaroslav.bachorik@oracle.com</a>>:<br> <blockquote \
class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc \
solid;padding-left:1ex"> On 5.8.2015 16:53, Eamonn McManus wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc \
solid;padding-left:1ex"> <br>
I would remove the spec changes about the limit on the domain length,<br>
which are a property of this particular implementation. It's perfectly<br>
reasonable to blow up if the domain length is > 536,870,911, but<br>
there's no reason for it to be in the spec.<br>
</blockquote>
<br>
<br>
Well, CCC board had a different opinion. That's why this limit is<br>
documented<br>
in the spec.<br>
<br>
-JB-<br>
<br>
<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc \
solid;padding-left:1ex"> Éamonn<br>
<br>
<br>
2015-08-05 4:48 GMT-07:00 Jaroslav Bachorik<br>
<<a href="mailto:jaroslav.bachorik@oracle.com" \
target="_blank">jaroslav.bachorik@oracle.com</a>>:<br> <blockquote \
class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc \
solid;padding-left:1ex"> <br>
Eamonn, Daniel,<br>
<br>
thanks for the comments.<br>
<br>
I've updated the webrev to address them. Also, I've added a test to<br>
exercise<br>
the boolean flag en-/decoding in ObjectName.<br>
<br>
<a href="http://cr.openjdk.java.net/~jbachorik/8041565/webrev.03" rel="noreferrer" \
target="_blank">http://cr.openjdk.java.net/~jbachorik/8041565/webrev.03</a><br> <br>
<br>
Cheers,<br>
<br>
-JB-<br>
<br>
<br>
On 4.8.2015 23:02, Daniel Fuchs wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc \
solid;padding-left:1ex"> <br>
<br>
Hi Jaroslav,<br>
<br>
379 * This field encodes _domain_pattern,<br>
_property_list_pattern<br>
and<br>
380 * _property_value_pattern booleans.<br>
<br>
If I'm not mistaken it also encodes the domain length.<br>
<br>
<br>
1072 if ((l & FLAG_MASK) > 0 ) {<br>
<br>
Although it is not expected that 'l' could be negative, it might be<br>
better to write this test as:<br>
<br>
if ((l & FLAG_MASK) != 0 ) {<br>
<br>
(+ I agree with Éamonn that l is not a great parameter name -<br>
nice to<br>
see you back Éamonn :-)) best regards, -- daniel On 8/4/15 4:20 PM,<br>
Jaroslav Bachorik wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc \
solid;padding-left:1ex"> <br>
<br>
Hi, reviving this review. On 14.4.2015 16:58, Jaroslav Bachorik<br>
wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc \
solid;padding-left:1ex"> <br>
<br>
On 14.4.2015 14:56, Daniel Fuchs wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc \
solid;padding-left:1ex"> <br>
<br>
Hi Jaroslav, I like this change, but it does introduce an<br>
incompatibility, so it probably needs a CCC and some release<br>
notes.<br>
For instance, this test passes with the current version of<br>
ObjectName: public class StringLengthTest { final static int<br>
smax = Short.MAX_VALUE; final static int smore = smax + 126;<br>
public static void main(String[] args) throws<br>
MalformedObjectNameException { char[] chars = new<br>
char[smore]; Arrays.fill(chars, 0, smax, 'a');<br>
Arrays.fill(chars, smax, smore, 'b');<br>
System.out.println(new ObjectName(new String(chars), "type",<br>
"Test")); } } I'm not sure what it will do with your<br>
changes :-)<br>
</blockquote>
<br>
<br>
It will fail with assert (if enabled). Or truncate the domain<br>
name, I<br>
suppose.<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc \
solid;padding-left:1ex"> <br>
<br>
With that in mind I believe you should consider throwing<br>
InternalError - or IllegalArgumentException - instead of using<br>
'assert' statements.<br>
</blockquote>
<br>
<br>
This would probably be better.<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc \
solid;padding-left:1ex"> <br>
<br>
BTW have you run the JCK?<br>
</blockquote>
<br>
<br>
Yes. All passed. I don't think JCK is testing for unrealistic<br>
values<br>
> ) I don't see how one could come up with a domain name 32767<br>
characters long.<br>
</blockquote>
<br>
<br>
The proposed change has passed the CCC review. In case the domain<br>
name<br>
is longer than Integer.MAX_VALUE/4 a MalformedObjectNameException<br>
will<br>
be thrown. All the JMX tests and JCK are still passing.<br>
<a href="http://cr.openjdk.java.net/~jbachorik/8041565/webrev.02" rel="noreferrer" \
target="_blank">http://cr.openjdk.java.net/~jbachorik/8041565/webrev.02</a> -JB-<br> \
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc \
solid;padding-left:1ex"> <br>
<br>
-JB-<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc \
solid;padding-left:1ex"> <br>
<br>
best regards, -- daniel On 13/04/15 17:07, Jaroslav Bachorik<br>
wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc \
solid;padding-left:1ex"> <br>
<br>
Hi Roger, On 13.4.2015 16:07, Roger Riggs wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc \
solid;padding-left:1ex"> <br>
<br>
Hi Jaroslav, Minor comments: 1488+: In forms like:<br>
_pattern_flag &= (~PROPLIST_PATTERN & 0xff);" The &0xff seems<br>
unnecessary since the store is to a byte field.<br>
</blockquote>
<br>
<br>
Fixed: <a href="http://cr.openjdk.java.net/~jbachorik/8041565/webrev.01" \
rel="noreferrer" target="_blank">http://cr.openjdk.java.net/~jbachorik/8041565/webrev.01</a><br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc \
solid;padding-left:1ex"> <br>
<br>
1644: the ? and : operators should be surrounded by spaces.<br>
There<br>
are other style issues, such as then statements on the same<br>
line<br>
as the 'if' but that may be beyond the scope of this change.<br>
</blockquote>
<br>
<br>
I know but these style issue have been around since the file was<br>
first committed. I didn't address them because it didn't feel<br>
right<br>
to mix style changes with the actual functionality. Cheers, -JB-<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc \
solid;padding-left:1ex"> <br>
<br>
Otherwise looks fine (as a 9 reviewer, but not specifically a<br>
serviceability reviewer). Thanks, Roger On 4/13/2015 5:43 AM,<br>
Jaroslav Bachorik wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc \
solid;padding-left:1ex"> <br>
<br>
Please, review the following change Issue :<br>
<a href="https://bugs.openjdk.java.net/browse/JDK-8041565" rel="noreferrer" \
target="_blank">https://bugs.openjdk.java.net/browse/JDK-8041565</a> Webrev:<br> <a \
href="http://cr.openjdk.java.net/~jbachorik/8041565/webrev.00" rel="noreferrer" \
target="_blank">http://cr.openjdk.java.net/~jbachorik/8041565/webrev.00</a> In<br> \
situations when there are 10s of thousands ObjectNname<br> instances<br>
around (enterprise setups etc.) the 3 separate internal<br>
boolean<br>
fields can lead to a noticeable memory waste. Adding insult to<br>
the injury, with the current field layout it is necessary to<br>
align the instances by 4 bytes. When using JOL<br>
(<a href="http://openjdk.java.net/projects/code-tools/jol/" rel="noreferrer" \
target="_blank">http://openjdk.java.net/projects/code-tools/jol/</a>) to<br> inspect \
the<br> object layout we can see this: Before optimization<br>
(JDK8u40): ---<br>
javax.management.ObjectName object internals: OFFSET SIZE TYPE<br>
DESCRIPTION VALUE 0 12 (object header)| N/A 12 4<br>
int<br>
ObjectName._domain_length N/A 16 1 boolean<br>
ObjectName._domain_pattern N/A 17 1 boolean<br>
ObjectName._property_list_pattern N/A 18 1 boolean<br>
ObjectName._property_value_pattern N/A 19 1<br>
(alignment/padding gap) N/A 20 4 String<br>
ObjectName._canonicalName N/A 24 4 Property[]<br>
ObjectName._kp_array N/A 28 4 Property[]<br>
ObjectName._ca_array N/A 32 4 Map<br>
ObjectName._propertyList<br>
N/A 36 4 (loss due to the next object alignment) Instance<br>
size: 40 bytes (estimated, the sample instance is not<br>
available)<br>
Space losses: 1 bytes internal + 4 bytes external = 5 bytes<br>
total<br>
{noformat} After optimization (JDK9 internal build): ---<br>
javax.management.ObjectName object internals: OFFSET SIZE<br>
TYPE<br>
DESCRIPTION VALUE 0 12 (object header) N/A 12 2<br>
short<br>
ObjectName._domain_length N/A 14 1 byte<br>
ObjectName._pattern_flag N/A 15 1 (alignment/padding gap)<br>
N/A 16 4 String ObjectName._canonicalName N/A 20 4<br>
Property[] ObjectName._kp_array N/A 24 4 Property[]<br>
ObjectName._ca_array N/A 28 4 Map<br>
ObjectName._propertyList<br>
N/A Instance size: 32 bytes (estimated, the sample instance is<br>
not available) Space losses: 1 bytes internal + 0 bytes<br>
external<br>
= 1 bytes total After optimization we can save 8 bytes per<br>
instance which can translate to very interesting numbers on<br>
large<br>
installations. To achieve this the domain name length is<br>
set to<br>
be *short* instead of *int* and the three booleans kept for<br>
the<br>
performance purposes are encoded into one byte value (as<br>
proposed<br>
by the reporter, Jean-Francois Denise). All the regression and<br>
JCK tests are passing after this change. Thanks, -JB-<br>
</blockquote></blockquote></blockquote></blockquote></blockquote></blockquote></blockquote>
<br>
<br>
<br>
</blockquote></blockquote>
<br>
</blockquote></blockquote>
<br>
</blockquote>
<br>
</blockquote>
<br>
</blockquote></div>
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic