RFR: 8238665: Add JFR event for direct memory statistics

[Erik Gahlin]

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 at 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.gahlin at 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.ddh at alibaba-inc.com <mailto: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/ <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 <mailto:erik.gahlin at oracle.com>>
>> Send Time:2020年3月3日(星期二) 04:08
>> To:董登辉(卓昂) <denghui.ddh at alibaba-inc.com <mailto:denghui.ddh at alibaba-inc.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 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
>> 
>