New allocation profiler

[Aleksey Shipilev]

Hi again,

Thanks for making the changes!

On 04/15/2015 01:03 PM, Vladimir Sitnikov wrote:
>> having no way to be notified about thread creation and destruction
> 
> I see at least two approaches there:
> 1) Java agent + hotswap Thread.init / Thread.exit.
> 2) lurk on java.lang.Thread#nextThreadID to detect and warn user on
> non-captured threads (e.g. separate output counter with "number of
> non-captured threads")

First approach does not readily fit into current
InternalProfiler/ExternalProfiler split. Only ExternalProfilers are able
to inject command line arguments, and only InternalProfilers have the
access to benchmark VM MXBeans.

Second approach is a plausible safety net, but I'd think it sets us up
for false positives: what if the new threads are allocating a few
objects, and do not skew the allocation rate when merged with
heavy-allocating benchmark threads? Therefore, I would think
cross-referencing the allocation data with GC churn is a better check,
see below.

My inclination is to accept the patch in the current form, because it is
already useful. We can then improve the internals further, as time allows.


>> I wonder if we should fold this new profiler into -prof gc, that reports
>> churn rates. Users will then have a more complete picture of what is
>> going on:
> 
> I wonder if profilers can have arguments.
> Personally, I do not like that -prof gc prints lots of excessive
> information. Well, it is useful when you really want to know that, but
> I do not always want to know all the details.
> Throwing all kind of metaspaces to an unprepared user is not that good either.

The "common" way in to specialize profilers in JMH is system properties,
e.g. -Djmh.gc.reportChurn=false, if you want something like that.


> Something like "-prof gc" to print just basic allocation profile and
> "-prof gc=details" to include space breakdown would be nice.
> 
> I use separate "profiler label" to make output compact.

That UX improvement was on the table longer than I can remember, which
explains why there is no RFE recorded for it. There is now:
  https://bugs.openjdk.java.net/browse/CODETOOLS-7901387

You are welcome to contribute another patch! :)


>> a,b,c
> 
> Those a fair points, however I do not think you always want double check that.
> You do it once, then you run your benchmark with different parameters
> and you know that allocation profile is measured "properly".
> 
> That relates to "why don't we always use all the profilers?" question.
> Well, do you?

Not all profilers, no. But profilers that report the cross-checkable
metrics -- sure!

After sleeping on it, I am convinced allocation stats should be reported
in -prof gc. It would tell the whole story how the memory floats through
the runtime. The only piece left in current GC stats is the actual
allocation count, and your profiler addresses it nicely.

So (puts his Maintainer Hat on), please fold HotspotAllocationProfiler
into GCProfiler, if don't have strong objections against doing so.


>> Code comments:

>>  * Do we actually need static initializers? That will init the profiler
>> even when we don't use it. E.g. it will be inited on any
>> ProfilerFactory.getAvailableProfilers() call.
> 
> I just thought it would be nice to avoid duplication of getMethod stuff.
> Do you think NoClass...Errors would appear in corner cases?
> I've changed to catch(Throwable) in the clinit so the the error does
> not block the whole thing.

Ah, my argument was to avoid doing excess init work altogether when
profiler is not used. Storing the ThreadMXBean / Methods in the instance
fields seems better?


> Am I right "current thread" means "jmh infrastructure only"?
> The idea is to include allocations made by user-provided code only.

InternalProfilers are supposed to be called by JMH control threads now,
so yeah, filtering "current thread" is reasonable.


>>  * Returning -1 as "absent" result. Double.NaN is universally used to
>> denote the cases like that.
> 
> That's reasonable.
> ClassloaderProfiler and CompilerProfiler obviously use -1.

Oh, thanks, that's an issue. I'll address it:
 https://bugs.openjdk.java.net/browse/CODETOOLS-7901388


Thanks,
-Aleksey.