fix PcpMmvWriter memory leak #121
Conversation
|
An alternative, simpler, solution would be to just call |
|
@pwinckles I haven't heard from any of the Parfait developers recently, but let's give them a little more time. If we don't get a response though would you be interested in becoming a direct contributor to this git repo? |
What does that entail? I tested the alternative solution that simply clears |
|
@pwinckles I'll send you an invite to join the pcp github org - should be an email arriving shortly. There's a #parfait team on PCP slack channel I encourage you to join - see https://pcp.io/community.html - it's very quiet but some of the original Parfait developers may still be lurking there if you have further questions. |
The `perMetricsByteBuffers` map leaks in `PcpMmvWriter` as follows: 1. A new metric is added. 2. Skipping some plumbing, `PcpMmvWriter.stop()` is called, which clears `PcpMmvWriter.metricData`, which is a map of `PcpValueInfo` objects that describe all of the metrics. 3. All of the metrics are re-added with calls to `PcpMmvWriter.addMetricInfo()`, which creates new `PcpValueInfo` instances to repoplulate `PcpMmvWriter.metricData`. 4. `PcpMmvWriter.start()` is called, which creates a new `ByteBuffer`, and populates slices of the buffer into `PcpMmvWriter.perMetricByteBuffers`. This map is keyed off the new `PcpValueInfo` instances, and this is where it's leaking. This fixes the leak by clearing the map when `PcpMmvWriter.reset()` is called. This is safe because that method also clears `PcpMmvWriter.metricData`, which is assumed to be insync with `perMetricByteBuffers`. Clearing the map allows the old `PcpValueInfo` objects to be GC'd as well as the backing `DirectByteBuffer`.
|
Thanks! I did a force push on this branch to change it to the |
|
@natoscott I'm not in a particular rush or anything, as we're already using a forked version, but, since you added me to the github group, I'm uncertain as to what your process expectations are here. Shall I just ping the slack and wait and see for a few more weeks? |
|
@pwinckles I think since noone's commented by now, go ahead and merge. |
The
perMetricsByteBuffersmap leaks inPcpMmvWriteras follows:PcpMmvWriter.stop()is called, which clearsPcpMmvWriter.metricData, which is a map ofPcpValueInfoobjects that describe all of the metrics.PcpMmvWriter.addMetricInfo(), which creates newPcpValueInfoinstances to repoplulatePcpMmvWriter.metricData.PcpMmvWriter.start()is called, which creates a newByteBuffer, and populates slices of the buffer intoPcpMmvWriter.perMetricByteBuffers. This map is keyed off the newPcpValueInfoinstances, and this is where it's leaking.This fixes the leak by clearing the map when
PcpMmvWriter.reset()is called. This is safe because that method also clearsPcpMmvWriter.metricData, which is assumed to be insync withperMetricByteBuffers. Clearing the map allows the oldPcpValueInfoobjects to be GC'd as well as the backingDirectByteBuffer.