RFR: 8238665: Add JFR event for direct memory statistics

[Erik Gahlin]

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.ddh at 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.gahlin at oracle.com>
> Send Time:2020年3月3日(星期二) 04:08
> To:董登辉(卓昂) <denghui.ddh at alibaba-inc.com>
> Cc:hotspot-jfr-dev <hotspot-jfr-dev at 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 <https://bugs.openjdk.java.net/browse/JDK-8238665>
> Webrev: http://cr.openjdk.java.net/~ddong/8238665/webrev.01/ <http://cr.openjdk.java.net/~ddong/8238665/webrev.01/>
> 
> ------------------------------------------------------------------
> From:董登辉(卓昂) <denghui.ddh at alibaba-inc.com> <mailto:denghui.ddh at alibaba-inc.com>
> Send Time:2020年2月11日(星期二) 16:13
> To:Erik Gahlin <erik.gahlin at oracle.com> <mailto:erik.gahlin at oracle.com>
> Cc:hotspot-jfr-dev <hotspot-jfr-dev at openjdk.java.net> <mailto:hotspot-jfr-dev at 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/ <http://cr.openjdk.java.net/~ddong/8238665/webrev.01/>
> 
> On Feb 11, 2020, at 1:52 AM, Erik Gahlin <erik.gahlin at oracle.com <mailto:erik.gahlin at 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 <https://bugs.openjdk.java.net/browse/JDK-8238665>
> Webrev: http://cr.openjdk.java.net/~ddong/8238665/webrev.00/ <http://cr.openjdk.java.net/~ddong/8238665/webrev.00/>
> Test: test/jdk/jdk/jfr/event/runtime/TestDirectMemoryStatisticsEvent.java
> 
> Thanks
> Denghui Dong
>