Parent Controller Path Caching Optimization

[rajat315315]

Use case

Description

In JMeter, when a sampler fails with onErrorStartNextLoop enabled, or when loop logical actions (such as Break/Continue) are triggered, JMeter is forced to find the parent controller hierarchy from the current sampler up to the thread group root.

Currently, this is done by executing testTree.traverse() with a FindTestElementsUpToRootTraverser every time the loop action is triggered. For large test plans with deep nesting structures, this full DFS tree walk is highly redundant since the test tree's hierarchical structure does not change during the execution run. This leads to severe performance degradation and high CPU usage under high-failure loads.

Possible solution

Proposed Improvement

Add a thread-local parent controller path cache (Map<Sampler, List<Controller>>) inside JMeterThread. When resolving parent controllers:

1. Query the map to see if the sampler's parent controllers list has already been resolved.
2. If it is a cache miss, perform the testTree.traverse() once and store the result.
3. If it is a cache hit, return a lightweight mock traverser containing the cached controllers list directly, completely skipping the expensive DFS tree traversal.

Possible workarounds

No response

JMeter Version

6.0.0-SNAPSHOT

Java Version

No response

OS Version

No response

[rajat315315]

@vlsi @FSchumacher @dcoomber @milamberspace
Any possibility of a review?

[vlsi]

Can you share benchmark/profiling results to justify the change?

[rajat315315]

I have pasted the results in the Pull Request.

[milamberspace]

Review summary for the proposed optimization

The implementation in #6721 has been reviewed — disposition: changes requested (review). The caching idea is sound (a sampler's path to root is static during thread execution), but the current implementation has a blocking correctness issue plus a testing gap:

1. Cache key semantics (blocking). The cache is a HashMap<Sampler, List<Controller>>, so it keys on equals/hashCode — which AbstractTestElement defines by value (propMap). The original tree walk instead identifies the node by reference identity (node == nodeToFind in FindTestElementsUpToRootTraverser). Two distinct-but-equal samplers (e.g. copy-pasted requests) would collide and receive the wrong parent-controller path on error; additionally, a sampler's properties mutate at runtime, making it an unstable hash key. Suggested fix: IdentityHashMap, which restores the original identity semantics.

2. No regression test. The change alters behaviour on a correctness-sensitive path (loop control on sample error), so a cache-correctness regression would be silent. A test exercising the cache-hit branch — ideally with two equal-but-distinct samplers — is needed.

The performance motivation itself is not in question; only the correctness of the caching mechanism. Full details are in the inline review comments on #6721.

_{This summary was drafted by an AI-assisted tool (Apache Magpie) and may contain mistakes; an Apache JMeter maintainer has reviewed and confirmed it.}

[milamberspace]

@rajat315315 — heads up: the review of your PR #6721 requested some changes (see the summary above and the inline comments on the PR). The main blocker is the cache key semantics; happy to help once you've had a chance to look. Thanks for the contribution!

_{Message drafted by an AI-assisted tool (Apache Magpie), confirmed by a maintainer.}