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

Bug 66269: Add NoThreadClone to HeaderManager to reduce heap utilization #727

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
9 changes: 9 additions & 0 deletions bin/jmeter.properties
Expand Up @@ -488,6 +488,15 @@ remote_hosts=127.0.0.1
# RETURN_CUSTOM_STATUS.code=
# RETURN_CUSTOM_STATUS.message=

#---------------------------------------------------------------------------
# HTTP Header Manager configuration
#---------------------------------------------------------------------------
# Save memory by sharing HTTP Header Manager across threads.
# The drawback is that HTTP Header Manager's can't be modified in the runtime
# as all the threads would notice the change.
# If you need dynamic headers, consider using ${..} functions or variables
#http.header_manager.is_shareable=true

#---------------------------------------------------------------------------
# Results file configuration
#---------------------------------------------------------------------------
Expand Down
Expand Up @@ -71,7 +71,7 @@ public final void addNode(Object node, HashTree subTree) {
protected Object addNodeToTree(Object node) {
if ( (node instanceof TestElement) // Check can cast for clone
// Don't clone NoThreadClone unless honourNoThreadClone == false
&& !(honourNoThreadClone && node instanceof NoThreadClone)
&& !(honourNoThreadClone && node instanceof NoThreadClone && ((NoThreadClone) node).isShareable())
) {
Object newNode = ((TestElement) node).clone();
newTree.add(objects, newNode);
Expand Down
Expand Up @@ -24,4 +24,13 @@
*
*/
public interface NoThreadClone {
/**
* Allows element to indicate whether it can be shared between threads.
* By default, elements that implement {@code NoThreadClone} are shareable.
*
* @return true if the element is shareable between threads, so it won't be cloned
*/
default boolean isShareable() {
return true;
}
}
Expand Up @@ -503,7 +503,7 @@ public void setRunningVersion(boolean runningVersion) {
*/
@Override
public void recoverRunningVersion() {
if (this instanceof NoThreadClone) {
if (this instanceof NoThreadClone && ((NoThreadClone) this).isShareable()) {
// The element is shared between threads, so there's nothing to recover
// See https://github.com/apache/jmeter/issues/5875
return;
Expand Down
Expand Up @@ -29,18 +29,20 @@
import java.util.List;

import org.apache.jmeter.config.ConfigTestElement;
import org.apache.jmeter.engine.util.NoThreadClone;
import org.apache.jmeter.gui.Replaceable;
import org.apache.jmeter.testelement.TestElement;
import org.apache.jmeter.testelement.property.CollectionProperty;
import org.apache.jmeter.testelement.property.JMeterProperty;
import org.apache.jmeter.util.JMeterUtils;
import org.apache.jorphan.util.JOrphanUtils;

/**
* This class provides an interface to headers file to pass HTTP headers along
* with a request.
*
*/
public class HeaderManager extends ConfigTestElement implements Serializable, Replaceable {
public class HeaderManager extends ConfigTestElement implements Serializable, Replaceable, NoThreadClone {

private static final long serialVersionUID = 240L;

Expand All @@ -57,9 +59,24 @@ public HeaderManager() {
setProperty(new CollectionProperty(HEADERS, new ArrayList<>()));
}

@Override
public boolean isShareable() {
return JMeterUtils.getPropDefault("http.header_manager.is_shareable", true); // $NON-NLS-1$
}

private void assertMutable() {
if (isRunningVersion()) {
throw new IllegalStateException(
"Cannot modify HeaderManager " + getName() + " while test is running. " +
"If you need dynamic headers, prefer using ${...} functions in variables if you need dynamic headers."
);
}
}

/** {@inheritDoc} */
@Override
public void clear() {
assertMutable();
super.clear();
setProperty(new CollectionProperty(HEADERS, new ArrayList<>()));
}
Expand Down Expand Up @@ -160,13 +177,15 @@ public void addFile(String headerFile) throws IOException {
* @param h {@link Header} to add
*/
public void add(Header h) {
assertMutable();
getHeaders().addItem(h);
}

/**
* Add an empty header.
*/
public void add() {
assertMutable();
getHeaders().addItem(new Header());
}

Expand All @@ -176,6 +195,7 @@ public void add() {
* @param index index from the header to remove
*/
public void remove(int index) {
assertMutable();
getHeaders().remove(index);
}

Expand Down Expand Up @@ -224,6 +244,7 @@ public Header getFirstHeaderNamed(final String name) {
* @param name header name
*/
public void removeHeaderNamed(String name) {
assertMutable();
List<Integer> removeIndices = new ArrayList<>();
for (int i = getHeaders().size() - 1; i >= 0; i--) {
Header header = (Header) getHeaders().get(i).getObjectValue();
Expand Down Expand Up @@ -304,6 +325,7 @@ public HeaderManager merge(TestElement element) {

@Override
public int replace(String regex, String replaceBy, boolean caseSensitive) throws Exception {
assertMutable();
final CollectionProperty hdrs = getHeaders();
int totalReplaced = 0;
for (int i = 0; i < hdrs.size(); i++) {
Expand Down
2 changes: 2 additions & 0 deletions xdocs/changes.xml
Expand Up @@ -75,6 +75,7 @@ Summary
<h3>HTTP Samplers and Test Script Recorder</h3>
<ul>
<li><pr>5911</pr> Use Caffeine for caching HTTP headers instead of commons-collections4 LRUMap</li>
<li><pr>727</pr> Save memory when test plan contains lots of HTTP Header Manager elements by sharing the managers across threads (add NoThreadClone to HeaderManager)</li>
</ul>

<h3>Other samplers</h3>
Expand Down Expand Up @@ -108,6 +109,7 @@ Summary
Previously it cached the values based on iteration number only which triggered wrong results on concurrent executions.
The previous behavior can be temporary restored with <code>function.cache.per.iteration</code> property.
</li>
<li><pr></pr> Add <code>NoThreadClone#isShareable</code> method so the elements can dynamically decide if they need cloning or not</li>
</ul>

<ch_section>Non-functional changes</ch_section>
Expand Down
17 changes: 17 additions & 0 deletions xdocs/usermanual/properties_reference.xml
Expand Up @@ -602,6 +602,23 @@ JMETER-SERVER</source>
</property>
</properties>
</section>

<section name="&sect-num;.14 HTTP Header Manager configuration" anchor="http_header_manager">
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that &sect-num;.14 duplicates the number of the previous section, however, I believe we'll remove section numbers in #5915

<properties>
<property name="http.header_manager.is_shareable">
By default, HTTP Header Manager elements are shared between threads. It enables to save memory by not
cloning the elements for each thread, however, the drawback is that HTTP Header Manager's can't be
modified in the runtime.<br/>
However, HTTP Header Managers still support <code>${...}</code> functions and variables, so it is still
possible to have dynamic headers.
<br/>
Defaults to:
<code>true</code>
</property>
</properties>
</section>


<section name="&sect-num;.15 Results file configuration" anchor="results_file_config">
<properties>
<property name="jmeter.save.saveservice.output_format">
Expand Down