From 79d80e4d0d9fdded269b8aaa0d902c0981f22583 Mon Sep 17 00:00:00 2001 From: Jason Tedor Date: Thu, 17 Jan 2019 16:36:32 -0500 Subject: [PATCH] Only update response headers if we have a new one 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 79d7c3510c2d1..9c92d085b5bf3 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 @@ -328,7 +328,17 @@ public void addResponseHeader(final String key, final String value) { * @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); + } } /**