Looks good. Erik
> On 31 Mar 2020, at 18:22, Denghui Dong <denghui....@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.gah...@oracle.com> > Send Time:2020年3月31日(星期二) 19:18 > To:董登辉(卓昂) <denghui....@alibaba-inc.com> > Cc:Alan Bateman <alan.bate...@oracle.com>; hotspot-jfr-dev > <hotspot-jfr-...@openjdk.java.net>; core-libs-dev > <core-libs-dev@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....@alibaba-inc.com > <mailto:denghui....@alibaba-inc.com>> wrote: > > PING. > > @Erik Could you review it? > > Denghui Dong > > > > ------------------------------------------------------------------ > From:董登辉(卓昂) <denghui....@alibaba-inc.com > <mailto:denghui....@alibaba-inc.com>> > Send Time:2020年3月19日(星期四) 16:35 > To:Alan Bateman <alan.bate...@oracle.com <mailto:alan.bate...@oracle.com>>; > Erik Gahlin <erik.gah...@oracle.com <mailto:erik.gah...@oracle.com>> > Cc:hotspot-jfr-dev <hotspot-jfr-...@openjdk.java.net > <mailto:hotspot-jfr-...@openjdk.java.net>>; core-libs-dev > <core-libs-dev@openjdk.java.net <mailto:core-libs-dev@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.bate...@oracle.com > <mailto:alan.bate...@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 >
