-
Notifications
You must be signed in to change notification settings - Fork 4
Davids/optimize #763
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
base: aash/optimize
Are you sure you want to change the base?
Davids/optimize #763
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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<String, Writer> 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<String, Writer> lineProcessor; | ||
| private final StringBuilder lineBuffer = new StringBuilder(); | ||
|
|
||
| LineProcessingWriter(Writer delegate, BiConsumer<String, Writer> 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<String, Writer> lineProcessor; | ||
| private final StringBuilder lineBuffer = new StringBuilder(); | ||
|
|
||
| LineProcessingWriter(Writer delegate, BiConsumer<String, Writer> 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); | ||
|
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this is vectorized search over underlying bytes |
||
| if (newLineIndex == -1) { | ||
| lineBuffer.append(str, startIndex, off + len); | ||
| return; | ||
| } else { | ||
| lineBuffer.append(str, startIndex, newLineIndex + 1); | ||
|
Comment on lines
+164
to
+167
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this allows bulk array copy from |
||
| 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); | ||
| } | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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<String> 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."); | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -54,7 +54,7 @@ public final class LogParser<T> { | |
| 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*"; | ||
|
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. handles matching log lines including trailing newline |
||
| private static final Pattern WITCHCRAFT_LOG_PATTERN = Pattern.compile(WITCHCRAFT_LOG_PATTERN_STRING); | ||
|
|
||
| @SuppressWarnings("for-rollout:deprecation") | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
drive-by cleanup