Question about trace-IDs

[Roman Kennke]

Hi Markus,

Thanks for the exlanation. I'll leave atomic_inc() alone then ;-)

One more question: which tests would you recommend me to run before I
send the PR? Do you have a good testsuite that exercises JFR? (I don't
find anything in test/hotspot/jtreg ...)

Roman

> -----Original Message-----
> From: Roman Kennke <rkennke at redhat.com> 
> Sent: den 21 september 2020 13:48
> To: Markus Gronlund <markus.gronlund at oracle.com>; 
> hotspot-jfr-dev at openjdk.java.net
> Subject: Re: Question about trace-IDs
> 
> Hi Markus,
> 
> 
> 
> > Thanks for noticing some staleness here - remnants from having
> > used 
> > different atomic increment logic(s) in the past.
> > 
> > You can initialize all counters to 0, and also let LAST_TYPE_ID be 
> > used as-is without the + 1.
> > 
> 
> Thanks for confirming this!
> 
> While I'm at it, I could also replace the helper method atomic_inc()
> and its uses with Atomic::add() which has the advantage that it uses
> CPU instructions for atomic-add instead of a CAS-loop when the CPU
> has such an instruction. WDYT?
> 
> I have found using the templated versions of Atomic::inc() to be
> problematic on 32-bit platforms  (will compile but not link) as the
> traceid type is always 64-bit. Maybe Atomic::add() handles this
> better, but I don’t think it's worth modifying, and Atomic::cmpxchg()
> works on all platforms. 
> 
> 
> Roman
> 
> 
> > Thanks
> > Markus
> > 
> > -----Original Message-----
> > From: Roman Kennke <rkennke at redhat.com>
> > Sent: den 21 september 2020 12:39
> > To: hotspot-jfr-dev at openjdk.java.net
> > Subject: Question about trace-IDs
> > 
> > Hello all,
> > 
> > While studying the code around JFR trace-IDs, I came about
> > something 
> > that I find a little odd: trace-IDs for modules, packages and
> > class- 
> > loaders start at 2, while trace-IDs for threads start at 1 (and 
> > classes at LAST_TYPE_ID+1). The static counters (for modules,
> > packages 
> > and
> > classloaders) are initialized at 1, and the first call to
> > atomic_inc() increments this to 2 and this is the first value used.
> > I 
> > understand that 0 is the uninitialized value, but don't see that 1
> > has 
> > any special meaning.
> > 
> > See for example here:
> > https://urldefense.com/v3/__https://github.com/openjdk/jdk/blob/master
> > /src/hotspot/share/jfr/recorder/checkpoint/types/traceid/jfrTraceId
> > .cp
> > p*L65__;Iw!!GqivPVa7Brio!JGBhdXPwm7skiWvt6c0M2CXIXa_4CdjKsp6UKdp-b-
> > 19g
> > _WFUOsJlWJl32N6avBT21Ai$
> > 
> > vs
> > 
> > https://urldefense.com/v3/__https://github.com/openjdk/jdk/blob/master
> > /src/hotspot/share/jfr/recorder/checkpoint/types/traceid/jfrTraceId
> > .cp
> > p*L60__;Iw!!GqivPVa7Brio!JGBhdXPwm7skiWvt6c0M2CXIXa_4CdjKsp6UKdp-b-
> > 19g
> > _WFUOsJlWJl32N6ajNzFPfr$
> > 
> > Would it hurt to start counting trace-IDs for packages, modules
> > and 
> > classloaders at 1 too? (If not, I'd like to submit a patch - I'm
> > happy 
> > about something simple as my first contributions to JFR ;-) ).
> > 
> > Cheers,
> > Roman
> >