RFR: 8238665: Add JFR event for direct memory statistics

[Erik Gahlin]

Looks good. 

Erik

> On 31 Mar 2020, at 18:22, Denghui Dong <denghui.ddh at alibaba-inc.com> wrote:
> 
> Hi Erik,
> 
> IMO, one event type per pool is a better choice, because:
> 1. easy filtering and configuration as you said, and I don't think there will be too many buffer pool types in the future
> 2. some buffer pools may have extra fields, e.g. direct memory has max capacity limit.
> 
> webrev: http://cr.openjdk.java.net/~ddong/8238665/webrev.05/ <http://cr.openjdk.java.net/~ddong/8238665/webrev.05/>
> diff with webrev.04: http://cr.openjdk.java.net/~ddong/8238665/v4-v5.diff <http://cr.openjdk.java.net/~ddong/8238665/v4-v5.diff>
> 
> please review it, thanks!
> 
> Denghui Dong
> ------------------------------------------------------------------
> From:Erik Gahlin <erik.gahlin at oracle.com>
> Send Time:2020年3月31日(星期二) 19:18
> To:董登辉(卓昂) <denghui.ddh at alibaba-inc.com>
> Cc:Alan Bateman <Alan.Bateman at oracle.com>; hotspot-jfr-dev <hotspot-jfr-dev at openjdk.java.net>; core-libs-dev <core-libs-dev at openjdk.java.net>
> Subject:Re: RFR: 8238665: Add JFR event for direct memory statistics
> 
> Hi Denghui,
> 
> I think there should be one event type per pool, or there should be a text field specifying which pool it is. 
> 
> If we believe there will be many pools in the future, it may make sense for a field, otherwise I think an event type could be used. The benefit of one event type per pool is that there can be a description per pool and it also allows easy filtering and configuration.
> 
> If there would be four pools, they could all derive from an abstract event class and only add @Label and @Description per event subclass. If there would be forty pools, it would be too much noise and a field would be better.
> 
> Erik
> 
> On 31 Mar 2020, at 11:06, Denghui Dong <denghui.ddh at alibaba-inc.com <mailto:denghui.ddh at alibaba-inc.com>> wrote:
> 
> PING.
> 
> @Erik Could you review it? 
> 
> Denghui Dong
> 
> 
> 
> ------------------------------------------------------------------
> From:董登辉(卓昂) <denghui.ddh at alibaba-inc.com <mailto:denghui.ddh at alibaba-inc.com>>
> Send Time:2020年3月19日(星期四) 16:35
> To:Alan Bateman <Alan.Bateman at oracle.com <mailto:Alan.Bateman at oracle.com>>; 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>>; core-libs-dev <core-libs-dev at openjdk.java.net <mailto:core-libs-dev at openjdk.java.net>>
> Subject:Re: RFR: 8238665: Add JFR event for direct memory statistics
> 
> Hi,
> 
> On Mar 18, 2020, at 11:46 PM, Alan Bateman <Alan.Bateman at oracle.com <mailto:Alan.Bateman at oracle.com>> wrote:
> 
> 
> 
> On 13/03/2020 14:54, Denghui Dong wrote:
> Good suggestion, moved.
> Webrev: http://cr.openjdk.java.net/~ddong/8238665/webrev.03/ <http://cr.openjdk.java.net/~ddong/8238665/webrev.03/>
> This looks much better.
> 
> What would you think about renaming the JFR event to "DirectBufferStatistics"? The concern I have with the proposed naming is that it will be really awkward to extend it to support mapped buffers.
> 
> It’s ok for me.
> 
> 
> The implementation changes look okay, hopefully Erik will skim over them. One small suggestion for for ManagementFactoryHelper is that you can stream().collect(Collectors.toList()) to create the value for bufferPools. You could even change it to volatile so that getBufferMXBeans isn't a synchronized method.
> 
> 
> Make sense, updated.
> 
> Webrev: http://cr.openjdk.java.net/~ddong/8238665/webrev.04/ <http://cr.openjdk.java.net/~ddong/8238665/webrev.04/>
> 
> @Erik, could you help review it?
> 
> 
> -Alan.
> 
> Denghui Dong
>