Re: [8u] [JFR] 8246310 : Clean commented-out code about ModuleEntry andPackageEntry in JFR

Denghui Dong denghui.ddh at alibaba-inc.com
Tue Jun 2 11:26:36 UTC 2020


Thank you!

Webrev has been updated. Some other related code missing in the previous patch was also removed in the new patch.

Denghui Dong
------------------------------------------------------------------
From:Andrew Dinn <adinn at redhat.com>
Send Time:2020年6月2日(星期二) 18:20
To:董登辉(卓昂) <denghui.ddh at alibaba-inc.com>; jdk8u-dev <jdk8u-dev at openjdk.java.net>; Mario Torre <neugens at redhat.com>; Andrey Petushkov <andrey at azul.com>
Subject:Re: [8u] [JFR] 8246310 : Clean commented-out code about ModuleEntry andPackageEntry in JFR

Hi Denghui,

On 02/06/2020 08:22, Denghui Dong wrote:
> Hi,
> Could I have a review of this small patch that deletes the commented-out
> code about ModuleEntry andPackageEntry in JFR.
> Most of the deleted code is commented out, except 
> ```
> <XmlType name="Package" parameterType="const PackageEntry*" fieldType="const PackageEntry*"/>
> ```
> in metadata.xml.
> PackageEntry doesn't exist in 8u, so it's safe to delete this line.
> 
> Bugs: https://bugs.openjdk.java.net/browse/JDK-8246310
> Webrev: http://cr.openjdk.java.net/~ddong/8246310/webrev.00/
> Testing: jdk/jfr, no extra failure

Thanks. This is a good thing to get out of the way first.

The deletions all look fine. However, you missed a few things (I am
using line numbers in the /original/ file):

  jfrTypeSet.cpp:171-193 (method write__artifact__package())

  jfrTypeSet.cpp:295 (declaration of method package_symbols())

  jfrTypeSet.cpp:349-393 (template methods package_symbols() and
module_symbols())

  jfrWriterHost.inline.hpp:36 (include of typeArrayOop.inline.hpp)

I don't need to see another webrev,

regards,


Andrew Dinn
-----------
Senior Principal Software Engineer
Red Hat UK Ltd
Registered in England and Wales under Company Registration No. 03798903
Directors: Michael Cunningham, Michael ("Mike") O'Neill


More information about the jdk8u-dev mailing list