Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix memory leak for Transaction Controller using parent samples #6386

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

jgaalen
Copy link

@jgaalen jgaalen commented Dec 4, 2024

Description

Follow issue in this issue

Basically, there's a memory leak, specifically when transactions are set as a parent. All the sub results maintains their data until the end of the thread, which can build up significantly, especially when there are large (uncompressed) responses

This PR fixes this, and improves the memory usage of JMeter by a lot, even without using transactions. Because we're cleaning all the SamplerResult data after it's sent to all the listeners. There is no reason to maintain the sampler data for that specific thread. It now clears all fields with large data and removes the parent reference, so GC can actually clear it.

Motivation and Context

JMeter is very memory intensive and by far the majority of the memory is used for processed samplers, just holding memory until the end of the thread run.

How Has This Been Tested?

I've done a lot of testing, made fixes and looks like it is working out well and preserving all the sampleresults/httpsamplerresults to all the listeners.

I've made a couple of scripts to test with a lot of transactions, with and without parent usage. Some synthetic scripts with a lot of 1MB json requests

Screenshots (if appropriate):

This is a graph of original 5.6.3 version, with 30 transactions, using parents, and a single 1MB.json file in it. Running 30 threads and 1 second think time between the transaction (meaning a thread runs 30 seconds + response times)

image

Then ran the exact same jmx against this fix:

image Memory is a LOT lower.

Also ran the same tests without the usage of transaction as a parent:

5.6.3 baseline:
image

This fix:
image

Without the fix, we can see actual heap usage is hovering between 150MB and 200MB, with a max heap 420MB. With the fix, it is hovering between 50MB and 100MB with a max heap of 247MB. Also much more stable.

I've done some aggressive manual full GCs to see the real usage, and after the fix, the low end was significantly lower.

Here's a run against a realistic real website, 5 pages (transactions) with parent=true:

5.6.3 baseline:
image

After the fix:
image

Types of changes

It creates a deep copy of the sampler result and then forwards that to the listeners. After that we can safely remove all the values from the sampler.
It now only clears data after a root sampler is done, because if it's not on the root and generateParent is enabled, subresults keep building up.

Checklist:

  • My code follows the code style of this project.
  • I have updated the documentation accordingly.

Improve sampler cleanup during thread run
@jgaalen jgaalen changed the title Fix memory leak when transaction has parents Fix memory leak for Transaction Controller using parent samples Dec 4, 2024
@jgaalen
Copy link
Author

jgaalen commented Dec 10, 2024

memtest.jmx.zip
I've added a memtest script. Here you can play with enabling parent mode for transactions. When enabled, in 5.6.3 it runs out of 1GB heap, or just hangs just below it.
With the fix, heap is stable around 100MB.

transaction_mem_leak
I've also added a screenshot of a heap dump, which shows the top references to bytes. In a run with 1 thread, I kept the thread stall with a very long delay. Then with VisualVM I've triggered a GC first, the pulled a heap dump. Inside the script, there are 30 transactions, using parent=true, all with 1MB jsons (120KB after compression).
You can see that ALL 30 subResults are still in memory for the transaction. Because they are still linked from the transactionControllerConfigMap. It is fixed by removing the transaction from the pack and recreating it (so references in the buffers of the listeners are still ok and can be cleaned after a GC).

When doing the exact same with with parent=false, nothing is there, only the previousResult (which is good).

After the fix, we have the same result, only previousResult is still in memory, but all transaction data is cleared after it is finished.

@jgaalen
Copy link
Author

jgaalen commented Dec 10, 2024

@vlsi I've added an example script, added heap dump screenshot and improved the code. I only needed a workaround in the JMeterTest.java code or else I was getting serialization errors.

Maybe you can take a look further, I think this is a major improvement when it comes to memory consumption. My current workaround is to disable parents for transactions to keep memory at ease

@vlsi
Copy link
Collaborator

vlsi commented Dec 10, 2024

@jgaalen , thanks for the reproducer. I've got 1G dump, and I'll take a look into it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants