From ed297b7369e80a79ac5fe43164139ec271163933 Mon Sep 17 00:00:00 2001 From: Jason Tedor Date: Fri, 18 Jan 2019 08:20:05 -0500 Subject: [PATCH] Only update response headers if we have a new one (#37590) Currently when adding a response header, we do some de-duplication, and maybe drop the header on the floor if we have reached capacity. Yet, we still update the thread local tracking the response headers. This is really expensive because under the hood there is a shared reference that we synchronize on. In the case of a request processed across many shards in a tight loop, this contention can be detrimental to performance. We can avoid updating the thread local in these cases though, when the response header is duplicate of one that we have already seen, or when it's dropped on the floor. This commit addresses these performance issues by avoiding the unnecessary set. --- .../common/util/concurrent/ThreadContext.java | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/server/src/main/java/org/elasticsearch/common/util/concurrent/ThreadContext.java b/server/src/main/java/org/elasticsearch/common/util/concurrent/ThreadContext.java index 2c1011d1d9e..bd3507ef776 100644 --- a/server/src/main/java/org/elasticsearch/common/util/concurrent/ThreadContext.java +++ b/server/src/main/java/org/elasticsearch/common/util/concurrent/ThreadContext.java @@ -323,7 +323,17 @@ public final class ThreadContext implements Closeable, Writeable { * @param uniqueValue the function that produces de-duplication values */ public void addResponseHeader(final String key, final String value, final Function uniqueValue) { - threadLocal.set(threadLocal.get().putResponse(key, value, uniqueValue, maxWarningHeaderCount, maxWarningHeaderSize)); + /* + * Updating the thread local is expensive due to a shared reference that we synchronize on, so we should only do it if the thread + * context struct changed. It will not change if we de-duplicate this value to an existing one, or if we don't add a new one because + * we have reached capacity. + */ + final ThreadContextStruct current = threadLocal.get(); + final ThreadContextStruct maybeNext = + current.putResponse(key, value, uniqueValue, maxWarningHeaderCount, maxWarningHeaderSize); + if (current != maybeNext) { + threadLocal.set(maybeNext); + } } /**