RFR: 8251392: Consolidate Metaspace Statistics

[Coleen Phillimore]

On Thu, 29 Apr 2021 04:50:32 GMT, Thomas Stuefe <stuefe at openjdk.org> wrote:

> There is a lot of duplicate coding when it comes to the consumption of Metaspace Statistics.
> 
> Metaspace offers statistical APIs via `MetaspaceUtils::(reserved|committed|used)_(words|bytes)` for either the whole metaspace or non-class/class space separately. But many callers need some sort of combination of these values, which they keep in data holder structures which all are named "Metaspace" something and all do a very similar job. In particular, we have:
> 
> - MetaspaceSizesSnapshot (used for gc logs)
> - MetaspaceSnapshot (used in NMT)
> - MetaspaceSizes, MetaspaceSummary, JFRMetaspaceSummary (JFR)
> - MetaspaceCounters, CompressedClassSpaceCounters, MetaspacePerfCounters (jstat performance counters)
> - CompressedKlassSpacePool and MetaspacePool (used for MXBeans)
> 
> As much as possible coding should be unified.
> 
> In addition to that, all these callers share a common problem, in that retrieving individual statistical values via `MetaspaceUtils::(reserved|committed|used)_(words|bytes)` may yield inconsistent values. For example, "reserved" < "committed", or "committed" < "used". This is the cause of a variety of rare intermittent test errors in different areas, e.g. Performance counters (JDK-8163413, JDK-8153323), the gc log, the MemoryPoolMXBean and NMT (JDK-8237872).
> 
> -----------------------
> 
> This patch introduces two new data holder structures:
> - `MetaspaceStats` holds reserved/committed/used counter
> - `MetaspaceCombinedStats` holds an expression of these counters for class- and non-class metaspace each, as well as total counter.
> 
> Furthermore, the patch introduces two new APIs in MetaspaceUtils:
> - `MetaspaceStats get_statistics(type)`
> - `MetaspaceCombinedStats get_combined_statistics()`
> The former is used to get statistics for either non-class-space or class space; the latter gets statistics for the whole, including total values. Both APIs guarantee consistent values - reserved >= committed >= used - and never lock.
> 
> The patch uses these new data holders and these APIs to consolidate, clean up and simplify a lot of caller code, in addition to making that code resistant against inconsistent statistics:
> 
> - GC Log: class `MetaspaceSizesSnapshot` in *metaspace/metaspaceSizesSnapshot.cpp/hpp* had been used to hold metaspace statistics. Its default constructor sneakishly queried all values. `MetaspaceSizesSnapshot` has neem removed, caller code rewritten for MetaspaceCombinedStats and explicit value querying.
> 
> - NMT had class `MetaspaceSnapshot` to keep metaspace statistics and to print out baseline diffs. 
>    - `MetaspaceSizesSnapshot` was removed and replaced with `MetaspaceCombinedStats`.
>    - `MemSummaryDiffReporter::print_metaspace_diff()` has been modified: fixed a potential div-by-zero, removed the "free" statistics which is meaningless, and massaged the code a bit.
>    - Similar fixes have been done in `MemSummaryReporter::report_metadata()` (which, if backported, should fix JDK-8237872).
> 
> - jstat & co: Metaspace performance counters are realized by `MetaspaceCounters` and `CompressedClassSpaceCounters`, implementation resides in `MetaspacePerfCounters` (metaspace/metaspaceCounters.hpp/cpp). 
> 	- I completely removed `CompressedClassSpaceCounters` since there is no need for two APIs, class space is part of metaspace.
> 	- I simplified the counter coding a lot. I think the complexity was not needed.
> 	- The Metaspace counters are now retrieved in a consistent manner. This should take care of JDK-8163413, JDK-8153323 and similar bugs.
> 
> - `MetaspaceMemoryPool`, `CompressedClassSpaceMemoryPool` used in MxBeans: I changed the implementation to return consistent values. 
> 
> - JFR reports metaspace allocations (which, confusingly, uses a different terminology: "data_space" == non-class portion of metaspace). It used `MetaspaceSummary` to hold the statistics, which were composed of `MetaspaceSizes`.
> 	- I completely removed `MetaspaceSizes`
> 	- I rewrote `MetaspaceSummary` to use `MetaspaceCombinedStats`
> 
> - MetaspaceUtils::print_on() has been used to print metaspace statistics (by GCs). Function has been corrected to print consistent values.
> 
> - Added a simple gtest for the new APIs
> 
> Tests: 
> - manually executed hotspot tier1 tests
> - explicitly tested runtime/Metaspace, gc/metaspace, runtime/NMT
> - SAP tests ran for several weeks on all our platforms

This looks good to me.  Nice cleanup removing duplicated metaspace counting code!

-------------

Marked as reviewed by coleenp (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/3786