Discussion:
Review Request JDK-8212795: ThreadInfoCompositeData.toCompositeData fails to map ThreadInfo to CompositeData
(too old to reply)
Mandy Chung
2018-10-24 22:53:20 UTC
Permalink
This patch fixes the regression introduced by JDK-8198253 in 11.
It turns out that NetBeans uses the internal sun.management API to
convert ThreadInfo to CompositeData for performance reason.
ThreadInfoCompositeData::toCompositeData is no longer used
in JDK since JMX added the MXBean support in JDK 6. The fix for
JDK-8212197 resolves one issue reported [1] but not the bug in
ThreadInfoCompositeData::toCompositeData. Sven has filed an
issue in NetBeans to replace the use of JDK internal API.

Webrev:
http://cr.openjdk.java.net/~mchung/jdk12/webrevs/8212795/webrev.00/

Thanks
Mandy
[1]
http://mail.openjdk.java.net/pipermail/serviceability-dev/2018-October/025512.html
[2] https://issues.apache.org/jira/browse/NETBEANS-1478
Daniel Fuchs
2018-10-25 09:52:23 UTC
Permalink
Hi Mandy,

I agree that this looks more robust and will be better for
long term maintainability. I'm just surprised that

156 static CompositeType compositeType() {
157 return STACK_TRACE_ELEMENT_COMPOSITE_TYPE;
158 }

is no longer (or was never) needed in StackTraceElementCompositeData
when

146 static CompositeType v5CompositeType() {
147 return V5_COMPOSITE_TYPE;
148 }

appears to still be needed.

Otherwise, this looks good to me.

best regards,

-- daniel
Post by Mandy Chung
This patch fixes the regression introduced by JDK-8198253 in 11.
It turns out that NetBeans uses the internal sun.management API to
convert ThreadInfo to CompositeData for performance reason.
ThreadInfoCompositeData::toCompositeData is no longer used
in JDK since JMX added the MXBean support in JDK 6. The fix for
JDK-8212197 resolves one issue reported [1] but not the bug in
ThreadInfoCompositeData::toCompositeData. Sven has filed an
issue in NetBeans to replace the use of JDK internal API.
http://cr.openjdk.java.net/~mchung/jdk12/webrevs/8212795/webrev.00/
Thanks
Mandy
[1]
http://mail.openjdk.java.net/pipermail/serviceability-dev/2018-October/025512.html
[2] https://issues.apache.org/jira/browse/NETBEANS-1478
Mandy Chung
2018-10-25 15:53:57 UTC
Permalink
Post by Daniel Fuchs
Hi Mandy,
I agree that this looks more robust and will be better for
long term maintainability. I'm just surprised that
 156     static CompositeType compositeType() {
 157         return STACK_TRACE_ELEMENT_COMPOSITE_TYPE;
 158     }
is no longer (or was never) needed in StackTraceElementCompositeData
when
 146     static CompositeType v5CompositeType() {
 147         return V5_COMPOSITE_TYPE;
 148     }
appears to still be needed.
It's used by MonitorInfoCompositeInfo and ThreadInfoCompositeInfo to
build their CompositeType of older version.  For the current version, it
gets it from MappedMXBeanType.toOpenType and hence no need for
compositeType().
Post by Daniel Fuchs
Otherwise, this looks good to me.
Thanks for the review.

Mandy
Post by Daniel Fuchs
best regards,
-- daniel
Post by Mandy Chung
This patch fixes the regression introduced by JDK-8198253 in 11.
It turns out that NetBeans uses the internal sun.management API to
convert ThreadInfo to CompositeData for performance reason.
ThreadInfoCompositeData::toCompositeData is no longer used
in JDK since JMX added the MXBean support in JDK 6. The fix for
JDK-8212197 resolves one issue reported [1] but not the bug in
ThreadInfoCompositeData::toCompositeData. Sven has filed an
issue in NetBeans to replace the use of JDK internal API.
http://cr.openjdk.java.net/~mchung/jdk12/webrevs/8212795/webrev.00/
Thanks
Mandy
[1]
http://mail.openjdk.java.net/pipermail/serviceability-dev/2018-October/025512.html
[2] https://issues.apache.org/jira/browse/NETBEANS-1478
Sven Reimers
2018-10-25 17:09:09 UTC
Permalink
Hi,

jus tested the suggested fix against jdk12 head with NetBeans 10VC1 and
self sampling works as expected.

Thanks for your hard work.

Sven
Post by Daniel Fuchs
Hi Mandy,
I agree that this looks more robust and will be better for
long term maintainability. I'm just surprised that
156 static CompositeType compositeType() {
157 return STACK_TRACE_ELEMENT_COMPOSITE_TYPE;
158 }
is no longer (or was never) needed in StackTraceElementCompositeData
when
146 static CompositeType v5CompositeType() {
147 return V5_COMPOSITE_TYPE;
148 }
appears to still be needed.
It's used by MonitorInfoCompositeInfo and ThreadInfoCompositeInfo to build
their CompositeType of older version. For the current version, it gets it
from MappedMXBeanType.toOpenType and hence no need for compositeType().
Otherwise, this looks good to me.
Thanks for the review.
Mandy
best regards,
-- daniel
This patch fixes the regression introduced by JDK-8198253 in 11.
It turns out that NetBeans uses the internal sun.management API to
convert ThreadInfo to CompositeData for performance reason.
ThreadInfoCompositeData::toCompositeData is no longer used
in JDK since JMX added the MXBean support in JDK 6. The fix for
JDK-8212197 resolves one issue reported [1] but not the bug in
ThreadInfoCompositeData::toCompositeData. Sven has filed an
issue in NetBeans to replace the use of JDK internal API.
http://cr.openjdk.java.net/~mchung/jdk12/webrevs/8212795/webrev.00/
Thanks
Mandy
[1]
http://mail.openjdk.java.net/pipermail/serviceability-dev/2018-October/025512.html
[2] https://issues.apache.org/jira/browse/NETBEANS-1478
--
Sven Reimers

* Senior Expert Software Architect
* Java Champion
* NetBeans Dream Team Member: http://dreamteam.netbeans.org
* Community Leader NetBeans: http://community.java.net/netbeans
Desktop Java:
http://community.java.net/javadesktop
* JUG Leader JUG Bodensee: http://www.jug-bodensee.de
* Duke's Choice Award Winner 2009

* XING: https://www.xing.com/profile/Sven_Reimers8
* LinkedIn: http://www.linkedin.com/in/svenreimers
Mandy Chung
2018-10-25 17:12:09 UTC
Permalink
Thanks for verifying the fix, Sven.

Mandy
Post by Sven Reimers
Hi,
jus tested the suggested fix against jdk12 head with NetBeans 10VC1
and self sampling works as expected.
Thanks for your hard work.
Sven
Post by Daniel Fuchs
Hi Mandy,
I agree that this looks more robust and will be better for
long term maintainability. I'm just surprised that
 156     static CompositeType compositeType() {
 157         return STACK_TRACE_ELEMENT_COMPOSITE_TYPE;
 158     }
is no longer (or was never) needed in StackTraceElementCompositeData
when
 146     static CompositeType v5CompositeType() {
 147         return V5_COMPOSITE_TYPE;
 148     }
appears to still be needed.
It's used by MonitorInfoCompositeInfo and ThreadInfoCompositeInfo
to build their CompositeType of older version.  For the current
version, it gets it from MappedMXBeanType.toOpenType and hence no
need for compositeType().
Post by Daniel Fuchs
Otherwise, this looks good to me.
Thanks for the review.
Mandy
Post by Daniel Fuchs
best regards,
-- daniel
Post by Mandy Chung
This patch fixes the regression introduced by JDK-8198253 in 11.
It turns out that NetBeans uses the internal sun.management API to
convert ThreadInfo to CompositeData for performance reason.
ThreadInfoCompositeData::toCompositeData is no longer used
in JDK since JMX added the MXBean support in JDK 6. The fix for
JDK-8212197 resolves one issue reported [1] but not the bug in
ThreadInfoCompositeData::toCompositeData. Sven has filed an
issue in NetBeans to replace the use of JDK internal API.
http://cr.openjdk.java.net/~mchung/jdk12/webrevs/8212795/webrev.00/
<http://cr.openjdk.java.net/%7Emchung/jdk12/webrevs/8212795/webrev.00/>
Thanks
Mandy
[1]
http://mail.openjdk.java.net/pipermail/serviceability-dev/2018-October/025512.html
[2] https://issues.apache.org/jira/browse/NETBEANS-1478
--
Sven Reimers
* Senior Expert Software Architect
* Java Champion
* NetBeans Dream Team Member: http://dreamteam.netbeans.org
* Community Leader  NetBeans: http://community.java.net/netbeans
http://community.java.net/javadesktop
* JUG Leader JUG Bodensee: http://www.jug-bodensee.de
* Duke's Choice Award Winner 2009
* XING: https://www.xing.com/profile/Sven_Reimers8
* LinkedIn: http://www.linkedin.com/in/svenreimers
Sven Reimers
2018-10-25 17:16:24 UTC
Permalink
Hi Mandy,

will this be backported to 11?

Sven
Post by Mandy Chung
Thanks for verifying the fix, Sven.
Mandy
Hi,
jus tested the suggested fix against jdk12 head with NetBeans 10VC1 and
self sampling works as expected.
Thanks for your hard work.
Sven
Post by Daniel Fuchs
Hi Mandy,
I agree that this looks more robust and will be better for
long term maintainability. I'm just surprised that
156 static CompositeType compositeType() {
157 return STACK_TRACE_ELEMENT_COMPOSITE_TYPE;
158 }
is no longer (or was never) needed in StackTraceElementCompositeData
when
146 static CompositeType v5CompositeType() {
147 return V5_COMPOSITE_TYPE;
148 }
appears to still be needed.
It's used by MonitorInfoCompositeInfo and ThreadInfoCompositeInfo to
build their CompositeType of older version. For the current version, it
gets it from MappedMXBeanType.toOpenType and hence no need for
compositeType().
Otherwise, this looks good to me.
Thanks for the review.
Mandy
best regards,
-- daniel
This patch fixes the regression introduced by JDK-8198253 in 11.
It turns out that NetBeans uses the internal sun.management API to
convert ThreadInfo to CompositeData for performance reason.
ThreadInfoCompositeData::toCompositeData is no longer used
in JDK since JMX added the MXBean support in JDK 6. The fix for
JDK-8212197 resolves one issue reported [1] but not the bug in
ThreadInfoCompositeData::toCompositeData. Sven has filed an
issue in NetBeans to replace the use of JDK internal API.
http://cr.openjdk.java.net/~mchung/jdk12/webrevs/8212795/webrev.00/
Thanks
Mandy
[1]
http://mail.openjdk.java.net/pipermail/serviceability-dev/2018-October/025512.html
[2] https://issues.apache.org/jira/browse/NETBEANS-1478
--
Sven Reimers
* Senior Expert Software Architect
* Java Champion
* NetBeans Dream Team Member: http://dreamteam.netbeans.org
* Community Leader NetBeans: http://community.java.net/netbeans
http://community.java.net/javadesktop
* JUG Leader JUG Bodensee: http://www.jug-bodensee.de
* Duke's Choice Award Winner 2009
* XING: https://www.xing.com/profile/Sven_Reimers8
* LinkedIn: http://www.linkedin.com/in/svenreimers
--
Sven Reimers

* Senior Expert Software Architect
* Java Champion
* NetBeans Dream Team Member: http://dreamteam.netbeans.org
* Community Leader NetBeans: http://community.java.net/netbeans
Desktop Java:
http://community.java.net/javadesktop
* JUG Leader JUG Bodensee: http://www.jug-bodensee.de
* Duke's Choice Award Winner 2009

* XING: https://www.xing.com/profile/Sven_Reimers8
* LinkedIn: http://www.linkedin.com/in/svenreimers
Mandy Chung
2018-10-25 18:29:11 UTC
Permalink
I have requested backport to 11u and pending for approval.

Mandy
Post by Daniel Fuchs
Hi Mandy,
will this be backported to 11?
Sven
Thanks for verifying the fix, Sven.
Mandy
Post by Sven Reimers
Hi,
jus tested the suggested fix against jdk12 head with NetBeans
10VC1 and self sampling works as expected.
Thanks for your hard work.
Sven
On Thu, Oct 25, 2018 at 8:52 AM Mandy Chung
Post by Daniel Fuchs
Hi Mandy,
I agree that this looks more robust and will be better for
long term maintainability. I'm just surprised that
 156     static CompositeType compositeType() {
 157         return STACK_TRACE_ELEMENT_COMPOSITE_TYPE;
 158     }
is no longer (or was never) needed in
StackTraceElementCompositeData
when
 146     static CompositeType v5CompositeType() {
 147         return V5_COMPOSITE_TYPE;
 148     }
appears to still be needed.
It's used by MonitorInfoCompositeInfo and
ThreadInfoCompositeInfo to build their CompositeType of older
version.  For the current version, it gets it from
MappedMXBeanType.toOpenType and hence no need for
compositeType().
Post by Daniel Fuchs
Otherwise, this looks good to me.
Thanks for the review.
Mandy
Post by Daniel Fuchs
best regards,
-- daniel
Post by Mandy Chung
This patch fixes the regression introduced by JDK-8198253 in 11.
It turns out that NetBeans uses the internal sun.management API to
convert ThreadInfo to CompositeData for performance reason.
ThreadInfoCompositeData::toCompositeData is no longer used
in JDK since JMX added the MXBean support in JDK 6. The fix for
JDK-8212197 resolves one issue reported [1] but not the bug in
ThreadInfoCompositeData::toCompositeData. Sven has filed an
issue in NetBeans to replace the use of JDK internal API.
http://cr.openjdk.java.net/~mchung/jdk12/webrevs/8212795/webrev.00/
<http://cr.openjdk.java.net/%7Emchung/jdk12/webrevs/8212795/webrev.00/>
Thanks
Mandy
[1]
http://mail.openjdk.java.net/pipermail/serviceability-dev/2018-October/025512.html
[2] https://issues.apache.org/jira/browse/NETBEANS-1478
--
Sven Reimers
* Senior Expert Software Architect
* Java Champion
* NetBeans Dream Team Member: http://dreamteam.netbeans.org
* Community Leader  NetBeans: http://community.java.net/netbeans
http://community.java.net/javadesktop
* JUG Leader JUG Bodensee: http://www.jug-bodensee.de
* Duke's Choice Award Winner 2009
* XING: https://www.xing.com/profile/Sven_Reimers8
* LinkedIn: http://www.linkedin.com/in/svenreimers
--
Sven Reimers
* Senior Expert Software Architect
* Java Champion
* NetBeans Dream Team Member: http://dreamteam.netbeans.org
* Community Leader  NetBeans: http://community.java.net/netbeans
http://community.java.net/javadesktop
* JUG Leader JUG Bodensee: http://www.jug-bodensee.de
* Duke's Choice Award Winner 2009
* XING: https://www.xing.com/profile/Sven_Reimers8
* LinkedIn: http://www.linkedin.com/in/svenreimers
Loading...