From f910421974160446386a6df9d9fb33fa903221b6 Mon Sep 17 00:00:00 2001 From: David Schlosnagle Date: Mon, 22 Sep 2025 21:11:16 -0400 Subject: [PATCH] Optimize writing lines --- .../testreport/FormattingTestReporter.java | 127 +++++++++++------- .../FormattingTestReporterTest.java | 62 +++++++++ .../java/logging/format/LogParser.java | 2 +- 3 files changed, 138 insertions(+), 53 deletions(-) create mode 100644 gradle-witchcraft-logging/src/test/groovy/com/palantir/witchcraft/java/logging/gradle/testreport/FormattingTestReporterTest.java diff --git a/gradle-witchcraft-logging/src/main/groovy/com/palantir/witchcraft/java/logging/gradle/testreport/FormattingTestReporter.java b/gradle-witchcraft-logging/src/main/groovy/com/palantir/witchcraft/java/logging/gradle/testreport/FormattingTestReporter.java index 5ab9da0e..8824ad6c 100644 --- a/gradle-witchcraft-logging/src/main/groovy/com/palantir/witchcraft/java/logging/gradle/testreport/FormattingTestReporter.java +++ b/gradle-witchcraft-logging/src/main/groovy/com/palantir/witchcraft/java/logging/gradle/testreport/FormattingTestReporter.java @@ -18,6 +18,7 @@ // CHECKSTYLE:OFF +import com.google.common.annotations.VisibleForTesting; import com.palantir.witchcraft.java.logging.format.LogFormatter; import com.palantir.witchcraft.java.logging.format.LogParser; import java.io.File; @@ -43,10 +44,10 @@ final class FormattingTestReporter implements TestReporter { @Override public void generateReport(TestResultsProvider testResultsProvider, File file) { - if (delegate instanceof TestReporter) { - ((TestReporter) delegate).generateReport(new FormattingTestResultsProvider(testResultsProvider), file); - } else if (delegate instanceof HtmlTestReport) { - ((HtmlTestReport) delegate).generateReport(new FormattingTestResultsProvider(testResultsProvider), file); + if (delegate instanceof TestReporter testReporter) { + testReporter.generateReport(new FormattingTestResultsProvider(testResultsProvider), file); + } else if (delegate instanceof HtmlTestReport htmlTestReport) { + htmlTestReport.generateReport(new FormattingTestResultsProvider(testResultsProvider), file); } else { throw new IllegalArgumentException("Unknown delegate class: " + delegate.getClass()); } @@ -62,6 +63,7 @@ private static final class FormattingTestResultsProvider implements TestResultsP writer.write('\n'); } : Writable.NOP)); + private static final BiConsumer LINE_PROCESSOR = (line, outputWriter) -> { try { PARSER.tryParse(line) @@ -86,54 +88,10 @@ private static final class FormattingTestResultsProvider implements TestResultsP @Override public void writeAllOutput(long classId, TestOutputEvent.Destination destination, Writer writer) { - if (destination == TestOutputEvent.Destination.StdErr - || destination == TestOutputEvent.Destination.StdOut) { - delegate.writeAllOutput(classId, destination, new LineProcessingWriter(writer, LINE_PROCESSOR)); - } else { - delegate.writeAllOutput(classId, destination, writer); - } - } - - private static class LineProcessingWriter extends Writer { - private final Writer delegate; - private final BiConsumer lineProcessor; - private final StringBuilder lineBuffer = new StringBuilder(); - - LineProcessingWriter(Writer delegate, BiConsumer lineProcessor) { - this.delegate = delegate; - this.lineProcessor = lineProcessor; - } - - @Override - public void write(char[] cbuf, int off, int len) throws IOException { - for (int i = off; i < off + len; i++) { - char ch = cbuf[i]; - if (ch == '\n') { - processLine(); - } else { - lineBuffer.append(ch); - } - } - } - - @Override - public void flush() throws IOException { - delegate.flush(); - } - - @Override - public void close() throws IOException { - if (!lineBuffer.isEmpty()) { - processLine(); - } - delegate.close(); - } - - private void processLine() throws IOException { - String line = lineBuffer.toString(); - lineProcessor.accept(line, delegate); - delegate.write('\n'); - lineBuffer.setLength(0); + switch (destination) { + case StdErr, StdOut -> + delegate.writeAllOutput(classId, destination, new LineProcessingWriter(writer, LINE_PROCESSOR)); + default -> delegate.writeAllOutput(classId, destination, writer); } } @@ -172,4 +130,69 @@ public void close() throws IOException { delegate.close(); } } + + @VisibleForTesting + static class LineProcessingWriter extends Writer { + private final Writer delegate; + private final BiConsumer lineProcessor; + private final StringBuilder lineBuffer = new StringBuilder(); + + LineProcessingWriter(Writer delegate, BiConsumer lineProcessor) { + this.delegate = delegate; + this.lineProcessor = lineProcessor; + } + + @Override + public void write(char[] cbuf, int off, int len) { + synchronized (lock) { + for (int i = off; i < off + len; i++) { + char ch = cbuf[i]; + lineBuffer.append(ch); + if (ch == '\n') { + writeLine(); + } + } + } + } + + @Override + public void write(String str, int off, int len) { + synchronized (lock) { + for (int startIndex = off; startIndex < off + len; ) { + int newLineIndex = str.indexOf('\n', startIndex); + if (newLineIndex == -1) { + lineBuffer.append(str, startIndex, off + len); + return; + } else { + lineBuffer.append(str, startIndex, newLineIndex + 1); + writeLine(); + startIndex = newLineIndex + 1; + } + } + } + } + + @Override + public void flush() throws IOException { + synchronized (lock) { + delegate.flush(); + } + } + + @Override + public void close() throws IOException { + synchronized (lock) { + if (!lineBuffer.isEmpty()) { + writeLine(); + } + delegate.close(); + } + } + + private void writeLine() { + String line = lineBuffer.toString(); + lineProcessor.accept(line, delegate); + lineBuffer.setLength(0); + } + } } diff --git a/gradle-witchcraft-logging/src/test/groovy/com/palantir/witchcraft/java/logging/gradle/testreport/FormattingTestReporterTest.java b/gradle-witchcraft-logging/src/test/groovy/com/palantir/witchcraft/java/logging/gradle/testreport/FormattingTestReporterTest.java new file mode 100644 index 00000000..af7bae86 --- /dev/null +++ b/gradle-witchcraft-logging/src/test/groovy/com/palantir/witchcraft/java/logging/gradle/testreport/FormattingTestReporterTest.java @@ -0,0 +1,62 @@ +/* + * (c) Copyright 2025 Palantir Technologies Inc. All rights reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.palantir.witchcraft.java.logging.gradle.testreport; + +import static org.assertj.core.api.Assertions.assertThat; + +import com.palantir.witchcraft.java.logging.gradle.testreport.FormattingTestReporter.LineProcessingWriter; +import java.io.StringWriter; +import java.util.ArrayList; +import java.util.List; +import org.junit.jupiter.api.Test; + +class FormattingTestReporterTest { + @Test + void testWriteLines() throws Exception { + StringWriter delegate = new StringWriter(); + List lines = new ArrayList<>(); + try (LineProcessingWriter lineWriter = new LineProcessingWriter(delegate, (line, writer) -> { + assertThat(writer).isSameAs(delegate); + assertThat(line).isNotNull(); + lines.add(line); + delegate.write(line); + })) { + assertThat(delegate.toString()).isEmpty(); + lineWriter.write("Hello "); + assertThat(delegate.toString()).isEmpty(); + lineWriter.write("world".toCharArray()); + assertThat(delegate.toString()).isEmpty(); + lineWriter.write("!\nThis is"); + assertThat(delegate.toString()).isEqualTo("Hello world!\n"); + lineWriter.write(""); + assertThat(delegate.toString()).isEqualTo("Hello world!\n"); + lineWriter.write(" a test\n of the\nemer".toCharArray()); + assertThat(delegate.toString()).isEqualTo("Hello world!\nThis is a test\n of the\n"); + lineWriter.write(new char[0]); + assertThat(delegate.toString()).isEqualTo("Hello world!\nThis is a test\n of the\n"); + lineWriter.append("gency broadcasting system."); + assertThat(delegate.toString()).isEqualTo("Hello world!\nThis is a test\n of the\n"); + lineWriter.flush(); + assertThat(delegate.toString()).isEqualTo("Hello world!\nThis is a test\n of the\n"); + } + assertThat(delegate.toString()) + .isEqualTo("Hello world!\nThis is a test\n of the\nemergency broadcasting system."); + assertThat(lines) + .hasSize(4) + .containsExactly("Hello world!\n", "This is a test\n", " of the\n", "emergency broadcasting system."); + } +} diff --git a/witchcraft-logging-formatting/src/main/java/com/palantir/witchcraft/java/logging/format/LogParser.java b/witchcraft-logging-formatting/src/main/java/com/palantir/witchcraft/java/logging/format/LogParser.java index c225b2f8..481aefe9 100644 --- a/witchcraft-logging-formatting/src/main/java/com/palantir/witchcraft/java/logging/format/LogParser.java +++ b/witchcraft-logging-formatting/src/main/java/com/palantir/witchcraft/java/logging/format/LogParser.java @@ -54,7 +54,7 @@ public final class LogParser { SERVICE_V1, REQUEST_V2, EVENT_V2, METRIC_V1, TRACE_V1, AUDIT_V2, DIAGNOSTIC_V1, WRAPPED_V1); private static final String WITCHCRAFT_LOG_PATTERN_STRING = "\\{.*?\"type\"\\s*?:\\s*?\"(" - + LOG_TYPES.stream().map(Pattern::quote).collect(Collectors.joining("|")) + ")\".*?}"; + + LOG_TYPES.stream().map(Pattern::quote).collect(Collectors.joining("|")) + ")\".*?}\\s*"; private static final Pattern WITCHCRAFT_LOG_PATTERN = Pattern.compile(WITCHCRAFT_LOG_PATTERN_STRING); @SuppressWarnings("for-rollout:deprecation")