Added. Thanks, Denghui Dong ------------------------------------------------------------------ From:Erik Gahlin <erik.gah...@oracle.com> Send Time:2020年3月5日(星期四) 13:23 To:董登辉(卓昂) <denghui....@alibaba-inc.com>; core-libs-dev <core-libs-dev@openjdk.java.net> Cc:hotspot-jfr-dev <hotspot-jfr-...@openjdk.java.net> Subject:Re: RFR: 8238665: Add JFR event for direct memory statistics
Hi Denghui, The copyright headers should have the classpath exception, similar to other files in the same directory. If somebody in core-libs could review this, especially the modification of src/java.base/share/classes/module-info.java, I can the push this. Thanks Erik On 5 Mar 2020, at 03:44, Denghui Dong <denghui....@alibaba-inc.com> wrote: Hi Erik, Updated. Webrev: http://cr.openjdk.java.net/~ddong/8238665/webrev.01/ Thanks Denghui Dong ------------------------------------------------------------------ From:Erik Gahlin <erik.gah...@oracle.com> Send Time:2020年3月5日(星期四) 06:33 To:董登辉(卓昂) <denghui....@alibaba-inc.com> Cc:hotspot-jfr-dev <hotspot-jfr-...@openjdk.java.net> Subject:Re: RFR: 8238665: Add JFR event for direct memory statistics Hi Denghui, Looking through you patch again. Could you add copyright headers, and change “5000 ms” to “5 s” in the .jfc files. When you reply with the updated patch, could you please include core-libs-dev@openjdk.java.net in the reply since you are modifying exports in java.base. Thanks Erik On 4 Mar 2020, at 20:35, Erik Gahlin <erik.gah...@oracle.com> wrote: Hi Denghui, I have tested your fix and it seems to work. TestDefaultConfiguration takes into account the event is periodic, so it doesn't complain as I expected. Looks good. I can sponsor this. Thanks Erik On 3 Mar 2020, at 07:14, Denghui Dong <denghui....@alibaba-inc.com> wrote: Hi Erik, I agree with you that the default interval should be changed to 5s, the webrev (http://cr.openjdk.java.net/~ddong/8238665/webrev.01/) has been updated. No need to modify TestDefaultConfigurations.java, it is passed. Testing (Linux release/slow debug build): jdk/jfr, all passed except TestNativeLibrariesEvent and TestNetworkUtilizationEvent(unstable). And these two test failures were not caused by this patch. And there were some test errors in slow debug build, I have checked the error test list and confirm they are also error in slow debug build without this patch, such as TestRecordedFrame. There are two new files in this patch, should I add copyright header to them? Thanks Denghui Dong ------------------------------------------------------------------ From:Erik Gahlin <erik.gah...@oracle.com> Send Time:2020年3月3日(星期二) 04:08 To:董登辉(卓昂) <denghui....@alibaba-inc.com> Cc:hotspot-jfr-dev <hotspot-jfr-...@openjdk.java.net> Subject:Re: RFR: 8238665: Add JFR event for direct memory statistics Hi Denghui, Thanks for the explanation. I think the default interval could be increased to once every 5 seconds. Have you run the all the tests in test/jdk/jdk/jfr? I would expect TestDefaultConfigurations to have failed unless modified. Thanks Erik On 2020-02-26 14:40, 董登辉(卓昂) wrote: PING: Could you review it? Bug: https://bugs.openjdk.java.net/browse/JDK-8238665 Webrev: http://cr.openjdk.java.net/~ddong/8238665/webrev.01/ ------------------------------------------------------------------ From:董登辉(卓昂) <denghui....@alibaba-inc.com> Send Time:2020年2月11日(星期二) 16:13 To:Erik Gahlin <erik.gah...@oracle.com> Cc:hotspot-jfr-dev <hotspot-jfr-...@openjdk.java.net> Subject:Re: RFR: 8238665: Add JFR event for direct memory statistics Hi Erik, Thanks for the review. See answers embedded. New webrev: http://cr.openjdk.java.net/~ddong/8238665/webrev.01/ On Feb 11, 2020, at 1:52 AM, Erik Gahlin <erik.gah...@oracle.com> wrote: Hi Denghiu, Can you explain when this event can be useful, i.e. when troubleshooting x etc.? This makes it easier to determine if the event should be on by default, if it is worth the increased startup cost that the instrumentation creates and what a reasonable period for the event should be. Many Java middleware(e.g. Netty) and applications use direct memory for achieving higher performance, if java process use too much direct memory, some exceptions will occur, such as oom-killer. Hence, it’s necessary to monitor it, and we use JMX to monitor it in our production environment currently. src/jdk.jfr/share/classes/jdk/jfr/events/DirectMemoryStatisticsEvent.java: @Label("TotalCapacity") -> @Label("Total Capacity") @Label("MemoryUsed") -> @Label("Memory Used”) Updated What is meant by 'limit'? Could we have a better field name and/or description that explains what this fields contains, because it is not obvious from that name. Fields that contains bytes, should have the DataAmount; annotation. What is the value if -XX:MaxDirectMemorySize has not been set? limit -> maxCapacity if -XX:MaxDirectMemorySize has not been set, the value of Runtime.getRuntime().maxMemory() will be used. Can you make the event class final. Updated @StackTrace(false) should not be needed. Removed test/jdk/jdk/jfr/event/runtime/TestDirectMemoryStatisticsEvent.java: Use try-with-resources on the Recording object. Updated Thanks Denghui Dong Thanks Erik On 2020-02-07 06:24, Denghui Dong wrote: Hi, Could I have a review of a change that adds JFR event for direct memory statistics Bug: https://bugs.openjdk.java.net/browse/JDK-8238665 Webrev: http://cr.openjdk.java.net/~ddong/8238665/webrev.00/ Test: test/jdk/jdk/jfr/event/runtime/TestDirectMemoryStatisticsEvent.java Thanks Denghui Dong
