This is an automated email from the ASF dual-hosted git repository. btellier pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/james-project.git
commit 1a45cbe249517d867cc1daa5220c117476dd41fa Author: Benoit Tellier <[email protected]> AuthorDate: Wed May 19 21:47:42 2021 +0700 JAMES-3587 Optimize MDCBuilder::build MDC::build represents 0.40% of CPU time according to flame graphs conducted during performance tests. - Avoids the allocation of MDCCloseable objects - Avoids two intermediate collects --- .../java/org/apache/james/util/MDCBuilder.java | 47 +++++-------------- .../java/org/apache/james/util/MDCBuilderTest.java | 53 ---------------------- 2 files changed, 11 insertions(+), 89 deletions(-) diff --git a/server/container/util/src/main/java/org/apache/james/util/MDCBuilder.java b/server/container/util/src/main/java/org/apache/james/util/MDCBuilder.java index aefb7f7..5255f07 100644 --- a/server/container/util/src/main/java/org/apache/james/util/MDCBuilder.java +++ b/server/container/util/src/main/java/org/apache/james/util/MDCBuilder.java @@ -21,7 +21,6 @@ package org.apache.james.util; import java.io.Closeable; import java.io.IOException; -import java.util.List; import java.util.Map; import java.util.Optional; import java.util.function.Supplier; @@ -30,7 +29,6 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.slf4j.MDC; -import com.github.steveash.guavate.Guavate; import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableList; @@ -72,28 +70,6 @@ public class MDCBuilder { private static final Logger LOGGER = LoggerFactory.getLogger(MDCBuilder.class); - public static class Closeables implements Closeable { - private final List<Closeable> closeables; - - public Closeables(List<Closeable> closeables) { - Preconditions.checkNotNull(closeables); - this.closeables = ImmutableList.copyOf(closeables); - } - - @Override - public void close() throws IOException { - closeables.forEach(this::closeQuietly); - } - - private void closeQuietly(Closeable closeable) { - try { - closeable.close(); - } catch (IOException e) { - LOGGER.warn("Failed to close Closeable", e); - } - } - } - public static MDCBuilder create() { return new MDCBuilder(); } @@ -172,12 +148,12 @@ public class MDCBuilder { @VisibleForTesting Map<String, String> buildContextMap() { - return ImmutableMap.<String, String>builder() - .putAll(nestedBuilder.build() - .stream() - .map(MDCBuilder::buildContextMap) - .flatMap(map -> map.entrySet().stream()) - .collect(Guavate.toImmutableMap(Map.Entry::getKey, Map.Entry::getValue))) + ImmutableMap.Builder<String, String> result = ImmutableMap.builder(); + + nestedBuilder.build() + .forEach(mdcBuilder -> result.putAll(mdcBuilder.buildContextMap())); + + return result .putAll(contextMap.build()) .build(); } @@ -191,12 +167,11 @@ public class MDCBuilder { } public Closeable build() { - return new Closeables( - buildContextMap() - .entrySet() - .stream() - .map(entry -> MDC.putCloseable(entry.getKey(), entry.getValue())) - .collect(Guavate.toImmutableList())); + Map<String, String> contextMap = buildContextMap(); + contextMap.forEach(MDC::put); + + return () -> contextMap.keySet() + .forEach(MDC::remove); } } diff --git a/server/container/util/src/test/java/org/apache/james/util/MDCBuilderTest.java b/server/container/util/src/test/java/org/apache/james/util/MDCBuilderTest.java index dad7682..bf074fd 100644 --- a/server/container/util/src/test/java/org/apache/james/util/MDCBuilderTest.java +++ b/server/container/util/src/test/java/org/apache/james/util/MDCBuilderTest.java @@ -22,15 +22,9 @@ package org.apache.james.util; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatNullPointerException; -import java.io.Closeable; -import java.io.IOException; - import org.junit.jupiter.api.Test; -import com.google.common.collect.ImmutableList; - class MDCBuilderTest { - private static final String KEY_1 = "key1"; private static final String KEY_2 = "key2"; private static final String VALUE_1 = "value1"; @@ -91,51 +85,4 @@ class MDCBuilderTest { .containsEntry(KEY_1, VALUE_1) .containsEntry(KEY_2, VALUE_2); } - - @Test - void closeablesConstructorShouldThrowOnNullList() { - assertThatNullPointerException() - .isThrownBy(() -> new MDCBuilder.Closeables(null)); - } - - @Test - void closeablesCloseShouldNotThrowWhenEmpty() throws IOException { - new MDCBuilder.Closeables(ImmutableList.of()) - .close(); - } - - @Test - void closeablesCloseShouldCallAllUnderlyingCloseables() throws IOException { - ImmutableList.Builder<String> builder = ImmutableList.builder(); - - Closeable closeable1 = () -> builder.add(VALUE_1); - Closeable closeable2 = () -> builder.add(VALUE_2); - - new MDCBuilder.Closeables( - ImmutableList.of(closeable1, closeable2)) - .close(); - - assertThat(builder.build()) - .containsExactly(VALUE_1, VALUE_2); - } - - - @Test - void closeablesCloseShouldCallAllUnderlyingCloseablesWhenError() throws IOException { - ImmutableList.Builder<String> builder = ImmutableList.builder(); - - Closeable closeable1 = () -> builder.add(VALUE_1); - Closeable closeable2 = () -> { - throw new IOException(); - }; - Closeable closeable3 = () -> builder.add(VALUE_2); - - new MDCBuilder.Closeables( - ImmutableList.of(closeable1, closeable2, closeable3)) - .close(); - - assertThat(builder.build()) - .containsExactly(VALUE_1, VALUE_2); - } - } --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
