Memory Mode support: Adding memory mode, and implementing it for Asynchronous Instruments#5709
Conversation
# Conflicts: # sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/internal/state/AsynchronousMetricStorage.java
|
@jack-berg First PR 🎉 |
jack-berg
left a comment
There was a problem hiding this comment.
Thanks @asafm! This code will definitely reduce the readability of the code, but the performance improvement is so significant that I think its worth it to incur the additional complexity. I left a variety of comments, which are mostly small. I'm very supportive of the overall direction.
…MemoryMode.java Co-authored-by: jack-berg <[email protected]>
…MetricExporter.java Co-authored-by: jack-berg <[email protected]>
…MetricReader.java Co-authored-by: jack-berg <[email protected]>
…l/state/ObjectPool.java Co-authored-by: jack-berg <[email protected]>
… with other factory methods
| ? ImmutableMeasurement.doubleMeasurement( | ||
| start, measurement.epochNanos(), measurement.doubleValue(), processedAttributes) | ||
| : ImmutableMeasurement.longMeasurement( | ||
| start, measurement.epochNanos(), measurement.longValue(), processedAttributes); |
There was a problem hiding this comment.
Measurement withAttributes(Attributes);
Measurement withStartEpochNanos(long);Wouldn't having 2 methods result in making 2 immutable copies? Perhaps a single
Measurement update(Attributes, long);method would generate a bit less garbage
jack-berg
left a comment
There was a problem hiding this comment.
Very close IMO, just a few final cleanups!
|
Looks good to me @asafm! Thanks for sticking with it. I'm out for a long weekend but when I get back I can help get the build passing if still stuck. |
…moryMode.java Co-authored-by: jack-berg <[email protected]>
…moryMode.java Co-authored-by: jack-berg <[email protected]>
…moryMode.java Co-authored-by: jack-berg <[email protected]>
…moryMode.java Co-authored-by: jack-berg <[email protected]>
…moryMode.java Co-authored-by: jack-berg <[email protected]>
…moryMode.java Co-authored-by: jack-berg <[email protected]>
|
@jack-berg the build fails on API compare. and and |
|
Can you run |
|
@jack-berg Here's the output. What should I do next? Commit those files? |
Codecov ReportAttention:
... and 1 file with indirect coverage changes 📢 Thoughts on this report? Let us know!. |
Epic
This is the first PR out of several which implements #5105.
The problem
Issue #5105 describes it quite well, so I'll add a very short motivation.
Currently the SDK uses immutable objects during metric collection, where the bulk of the work is located at the
LeasedMetricProducer.The side effect of immutable objects is that they generate a lot of garbage collected objects. Given enough instruments and/or attribute sets, this can amount to such a substantial amount, the garbage collector will start to kick in more often, and for longer periods of time.
There are systems which are ultra sensitive to latency, like databases and messaging systems (e.g. Apache Pulsar). Those systems will have a problem using OpenTelemetry as it is today.
The solution
The SDK will have two modes of operation:
What does this PR include?
This PR is focused on adding support for reusability, and implementing it for Asynchronous Instruments.
MemoryModeenum dictating the two different modes.memoryModeproperty toMetricReader. This is used by the SDK collection process (in this PR only the Asynchronous Instrument related code) to switch between using immutable objects of reusable objects (from a pool).PeriodicMetricReaderwe've addedmemoryModeproperty toMetricExporteras well, so both the metric exporter can act upon it, and also the PeriodicMetricReader delegates the value ofmemoryModeto theMetricExporterit has.DoublePointDataandLongPointData. This are used to carry the current value for each (instrument, attributes) pair.Measurement, by changing it from an abstract class into an interface and creating two classes implementing it:ImmutableMeasurementandLeasedMeasurement.SdkObservableMeasurementhas been changed to object reuse if reusable memory mode is selected, for the active metric reader. Since it only createsMeasurementobject only as DTO for callingAsynchronousMetricStorage.record(measurement), I've added a singleLeasedMeasurementobject and keep re-using it across calls torecord()from the callbacks. It was named "leased" since theSdkObserableMeasurementleases the measurement toAsynchronousMetricStoragefor the duration of therecord()call - it should not store it hence "leased".Data structures for reusable objects
I've added an
ObjectPoolclass which as it sounds, allows you to borrow an object from the pool. If the pool is empty it uses anobjectCreatorfunction to generate one and return it. When the object is done with, instead of being garbage collected, it is returned back to the pool usingreturnObject(o).The pool it self must avoid any O(n) memory allocations upon borrow or return operations, hence using a simple array-based stack I wrote
ArrayBasedStack. I decided to write one instead of usingArrayDequeuesince I want in next PR improve the extra space it occupies (instead of x2 switching to changing double factor, or trim after x collections)Also I've added
PooledHashMap, which is a bucket-based HashMap implementation, which uses an object pool when it needs aMapEntryobject. This is needed primarily inAsynchronousMetricStorage.Storage
AsynchronousMetricStorage- here the bulk of the work was done in this PR. This is where all the callbackrecord()calls gets funneled into. The primary changes were:PooledHashMapinstead ofHashMapto reduceMapEntrymemory allocations which were O(n). The maps are used to store the recordings (attributes -> point) and in case of DELTA temporality, the previous data points.record(measurement), the measurement is now converted into a reusable data point, coming from a pool ofreusable data points, in the case of REUSABLE memory mode.
collect()is now a reusable list (array-based), in the case of REUSABLE memory mode. Array-based removes any internal wrapper objects allocations.collect()it return a list of reusable points, which needs to return back to the pool. We do so when the next collect() is called. We "save" the list returned, return each reusable data point back to the pool, and reset the list for another usage.The
*Aggregatorclasses were modified to allow for reusable data points to avoid allocations.Benchmark tests
Garbage Collection (memory allocation) benchmark
AsynchronousMetricStorageGarbageCollectionBenchmarkTestruns a JMH benchmark namedAsynchronousMetricStorageGarbageCollectionBenchmarkwithgcprofiler, which measures the numberof objects (and size) the garbage collector collected (unused objects).
The
AsynchronousMetricStorageGarbageCollectionBenchmarkbenchmark creates 10 asynchronous counters (any asynchronous instrument will do as the code path that was changes is almost the same for all async instrument types), and 1000 attribute sets. Each time the test runs, it callsflushwhich effectively calls the callback for each counter. Each such callback records a random number for each of the 1000 attribute sets. The result list ends up inNoOpMetricExporterwhich does nothing with it.This is repeated 100 times, collectively called operation in the statistics and each such Operation is repeated 20 times - known as Iteration.
Each such test is repeated, with a brand new JVM, for all combinations of memory mode (REUSBLE_DATA, IMMUTABLE_DATA) and aggregation temporality (CUMULATIVE, DELTA). This is done since each combination has a different code path.
The statistics of this run can be seen in the output of the test and also pasted here below:
If we focus only on the normalized rate:
You can see that for DELTA aggregation temporality, REUSABLE_DATA scores a rate of 257,078 [bytes/operation] compared with 271,877,200 [bytes/operation], which means REUSABLE_DATA memory mode reduced the size of
garbage collected objects by 99.9% compared with IMMUTABLE_DATA memory mode. Similar results (99.88%) exists for CUMULATIVE.
The test verifies that. Aside from the convenience of running this inside your code editor with one click, it protects from any future regression that can easily arise from a simple refactor or change.
Memory usage benchmark
IMMUTABLE_DATA "pays" with lots of one-off objects which almost immediately gets garbage collected. Memory is allocated to certain peak and then released. On the other hand, REUSABLE_DATA uses reusable objects, located in object pools, which stays in memory even after the collection finished. So it "pays" with constant memory usage. In a perfect implementation the peak memory consumption would almost be equal in both cases.
The object pools are not yet tuned, so they aggressively allocates more than what they need. In subsequent PRs I would like to minimize that effect, so the max heap for both is not far off each other.
You can run
AsynchronousCounterMemoryUsageBenchmarkmain()method to view the memory usage for each combination of aggregation temporality, memory mode, number of asynchronous instruments and number of attribute sets.The results are:
Cumulative
Delta
So you can see for example that for 50 counters and 100k attribute sets each, the maximum used memory is 958mb for reusable data and 581mb for immutable data, meaning 64% more memory needed. For 1 instrument, it's 20%. The reason it's bigger percentage as it has more attribute sets is because currently the object pool grows by 2 each time it runs out of capacity. This is something to be be tuned in next PRs.
opentelemetry-sdk-metrics.txtopentelemetry-sdk-testing.txtFuture PRs