Hi Jini,
It looks good in general.
Some minor comments though.
http://cr.openjdk.java.net/~jgeorge/8159127/webrev.01/hotspot/src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/classfile/ClassLoaderData.java.udiff.html
+ public ClassLoaderData next() {
+ Address classLoaderDataAddr = nextField.getValue(getAddress());
+ if (classLoaderDataAddr == null) {
+ return null;
+ }
+
+ return instantiateWrapperFor(classLoaderDataAddr);
+ }
+
+ public Klass getKlasses() {
+ Address klassesFieldAddr = klassesField.getValue(getAddress());
+ if (klassesFieldAddr == null) {
+ return null;
+ }
+ return (InstanceKlass)Metadata.instantiateWrapperFor(klassesFieldAddr);
+ }
It seems there is no need to check address for null as it is done
inside the instantiateWrapperFor call.
ClassLoaderData.java:
private static VirtualBaseConstructor<Metadata> metadataConstructor;
. . .
public static Metadata instantiateWrapperFor(Address addr) {
return metadataConstructor.instantiateWrapperFor(addr);
}
VirtualBaseConstructor.java:
public T instantiateWrapperFor(Address addr)throws WrongTypeException {
if (addr ==null) {
return null;
}
. . .
http://cr.openjdk.java.net/~jgeorge/8159127/webrev.01/hotspot/src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/utilities/HeapHprofBinWriter.java.frames.html
At the lines 522-547 it is possible to define and use the same callback class
for traversing both the SystemDictionary classes and the anonymous classes.
The same is applied to the lines 961-990.
But I leave it up to you to decide if it needs to be updated.
Thanks, Serguei On 12/13/16 08:34, Jini Susan George wrote:
Thank you, Dmitry. Could I get one more reviewer to take a look at this ?
-Jini.
-----Original Message-----
From: Dmitry Samersoff
Sent: Tuesday, December 13, 2016 2:48 PM
To: Jini Susan George; [email protected]
Subject: Re: RFR: JDK-8159127: hprof heap dumps broken for lambda
classdata
Jini,
Looks good to me.
-Dmitry
On 2016-12-13 11:55, Jini Susan George wrote:
Modified webrev:
http://cr.openjdk.java.net/~jgeorge/8159127/webrev.01/
Thanks,
Jini.
-----Original Message-----
From: Jini Susan George
Sent: Tuesday, December 13, 2016 10:09 AM
To: Dmitry Samersoff; [email protected]
Subject: RE: RFR: JDK-8159127: hprof heap dumps broken for lambda
classdata
Thank you, Dmitry. I will add the null check.
-jini
-----Original Message-----
From: Dmitry Samersoff
Sent: Monday, December 12, 2016 5:06 PM
To: Jini Susan George; [email protected]
Subject: Re: RFR: JDK-8159127: hprof heap dumps broken for lambda
classdata
Jini,
Looks good to me!
ClassLoaderData.java
85: Should we check klassField for null here?
-Dmitry
On 2016-12-12 13:31, Jini Susan George wrote:
Could I please get a review done for the following SA defect?
Bug: https://bugs.openjdk.java.net/browse/JDK-8159127
Webrev: http://cr.openjdk.java.net/~jgeorge/8159127/webrev.00/
Thanks,
- Jini Susan George
--
Dmitry Samersoff
Oracle Java development team, Saint Petersburg, Russia
* I would love to change the world, but they won't give me the sources.
--
Dmitry Samersoff
Oracle Java development team, Saint Petersburg, Russia
* I would love to change the world, but they won't give me the sources.
