Skip to content

Commit

Permalink
Refactor and add tests for optimized assembly traceback (#3949)
Browse files Browse the repository at this point in the history 
Follow-up to #3901:
* Renamed `Substring` to `StackLineView`
* Implemented tests for `StackLineView`
* Corrected `contains` and `startsWith` implementations
  • Loading branch information
@chemicL
chemicL authored Dec 4, 2024
1 parent 0a87988 commit 18379f3
Show file tree
Hide file tree
Showing 2 changed files with 114 additions and 32 deletions.
69 changes: 38 additions & 31 deletions reactor-core/src/main/java/reactor/core/publisher/Traces.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -120,14 +120,14 @@ static boolean isUserCode(String line) {
* from the assembly stack trace
*/
static String[] extractOperatorAssemblyInformationParts(String source) {
Iterator<Substring> traces = trimmedNonemptyLines(source);
Iterator<StackLineView> traces = trimmedNonemptyLines(source);

if (!traces.hasNext()) {
return new String[0];
}

Substring prevLine = null;
Substring currentLine = traces.next();
StackLineView prevLine = null;
StackLineView currentLine = traces.next();

if (currentLine.isUserCode()) {
// No line is a Reactor API line.
Expand Down Expand Up @@ -157,20 +157,20 @@ static String[] extractOperatorAssemblyInformationParts(String source) {
*
* @implNote This implementation attempts to minimize allocations.
*/
private static Iterator<Substring> trimmedNonemptyLines(String source) {
return new Iterator<Substring>() {
private int index = 0;
private static Iterator<StackLineView> trimmedNonemptyLines(String source) {
return new Iterator<StackLineView>() {
private int index = 0;
@Nullable
private Substring next = getNextLine();
private StackLineView next = getNextLine();

@Override
public boolean hasNext() {
return next != null;
}

@Override
public Substring next() {
Substring current = next;
public StackLineView next() {
StackLineView current = next;
if (current == null) {
throw new NoSuchElementException();
}
Expand All @@ -179,13 +179,13 @@ public Substring next() {
}

@Nullable
private Substring getNextLine() {
private StackLineView getNextLine() {
while (index < source.length()) {
int end = source.indexOf('\n', index);
if (end == -1) {
end = source.length();
}
Substring line = new Substring(source, index, end).trim();
StackLineView line = new StackLineView(source, index, end).trim();
index = end + 1;
if (!line.isEmpty()) {
return line;
Expand All @@ -196,63 +196,70 @@ private Substring getNextLine() {
};
}

// XXX: Explain.
private static final class Substring {
private final String str;
private final int start;
private final int end;
/**
* Provides optimized access to underlying {@link String} with common operations to
* view the stack trace line without unnecessary allocation of temporary
* {@code String} objects.
*/
static final class StackLineView {

private final String underlying;
private final int start;
private final int end;

Substring(String str, int start, int end) {
this.str = str;
StackLineView(String underlying, int start, int end) {
this.underlying = underlying;
this.start = start;
this.end = end;
}

Substring trim() {
StackLineView trim() {
int newStart = start;
while (newStart < end && str.charAt(newStart) <= ' ') {
while (newStart < end && underlying.charAt(newStart) <= ' ') {
newStart++;
}
int newEnd = end;
while (newEnd > newStart && str.charAt(newEnd - 1) <= ' ') {
while (newEnd > newStart && underlying.charAt(newEnd - 1) <= ' ') {
newEnd--;
}
return newStart == start && newEnd == end ? this : new Substring(str, newStart, newEnd);
return newStart == start && newEnd == end ? this : new StackLineView(
underlying, newStart, newEnd);
}

boolean isEmpty() {
return start == end;
}

boolean startsWith(String prefix) {
return str.startsWith(prefix, start);
boolean canFit = end - start >= prefix.length();
return canFit && underlying.startsWith(prefix, start);
}

boolean contains(String substring) {
int index = str.indexOf(substring, start);
return index >= 0 && index < end;
int index = underlying.indexOf(substring, start);
return index >= start && (index + (substring.length() - 1) < end);
}

boolean isUserCode() {
return !startsWith(PUBLISHER_PACKAGE_PREFIX) || contains("Test");
}

Substring withoutLocationSuffix() {
int linePartIndex = str.indexOf('(', start);
StackLineView withoutLocationSuffix() {
int linePartIndex = underlying.indexOf('(', start);
return linePartIndex > 0 && linePartIndex < end
? new Substring(str, start, linePartIndex)
? new StackLineView(underlying, start, linePartIndex)
: this;
}

Substring withoutPublisherPackagePrefix() {
StackLineView withoutPublisherPackagePrefix() {
return startsWith(PUBLISHER_PACKAGE_PREFIX)
? new Substring(str, start + PUBLISHER_PACKAGE_PREFIX.length(), end)
? new StackLineView(underlying, start + PUBLISHER_PACKAGE_PREFIX.length(), end)
: this;
}

@Override
public String toString() {
return str.substring(start, end);
return underlying.substring(start, end);
}
}
}
77 changes: 76 additions & 1 deletion reactor-core/src/test/java/reactor/core/publisher/TracesTest.java
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2018-2021 VMware Inc. or its affiliates, All Rights Reserved.
* Copyright (c) 2018-2024 VMware Inc. or its affiliates, 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.
Expand Down Expand Up @@ -146,4 +146,79 @@ public void shouldSanitizeTrue() {
assertThat(Traces.shouldSanitize("java.lang.reflect")).isTrue();
}

@Test
void stackLineViewSanityTest() {
String stackLine = "\treactor.core.publisher.Flux.filter(Flux.java:4209)\n";

int end = stackLine.indexOf('\n');
if (end == -1) {
System.out.println("No end-of-line");
end = stackLine.length();
}

Traces.StackLineView view = new Traces.StackLineView(stackLine, 0, end)
.trim();

assertThat(view.toString()).isEqualTo(stackLine.trim());
assertThat(view.isEmpty()).isFalse();
assertThat(view.isUserCode()).isFalse();
assertThat(view.contains("Flux.filter")).isTrue();
assertThat(view.startsWith("reactor.core.publisher.Flux")).isTrue();
}

@Test
void stackLineViewLimitsAreCheckedAtStart() {
String stackLine = "\treactor.core.publisher.Flux.filter(Flux.java:4209)\n";

Traces.StackLineView incompleteView =
new Traces.StackLineView(stackLine, stackLine.length() / 2, stackLine.length())
.trim();

assertThat(incompleteView.toString()).isEqualTo("ux.filter(Flux.java:4209)");
assertThat(incompleteView.contains(".filter")).isTrue();
assertThat(incompleteView.contains("ux.f")).isTrue();
assertThat(incompleteView.contains("lux.f")).isFalse();
assertThat(incompleteView.contains("09)")).isTrue();
assertThat(incompleteView.startsWith("ux.")).isTrue();
assertThat(incompleteView.startsWith("lux.")).isFalse();
assertThat(incompleteView.startsWith("ux.filter(Flux.java:4209)")).isTrue();
assertThat(incompleteView.startsWith("lux.filter(Flux.java:4209)")).isFalse();
}

@Test
void stackLineViewLimitsAreCheckedAtEnd() {
String stackLine = "\treactor.core.publisher.Flux.filter(Flux.java:4209)\n";

Traces.StackLineView incompleteView =
new Traces.StackLineView(stackLine, 0, stackLine.length() / 2)
.trim();

assertThat(incompleteView.toString()).isEqualTo("reactor.core.publisher.Fl");
assertThat(incompleteView.contains("Fl")).isTrue();
assertThat(incompleteView.contains("Flu")).isFalse();
assertThat(incompleteView.startsWith("reactor.core.publisher.Fl")).isTrue();
assertThat(incompleteView.startsWith("reactor.core.publisher.Flux")).isFalse();
}

@Test
void stackLineViewLocationSuffixGetsRemoved() {
String stackLine = "\treactor.core.publisher.Flux.filter(Flux.java:4209)\n";

Traces.StackLineView view =
new Traces.StackLineView(stackLine, 0, stackLine.length()).trim();

assertThat(view.withoutLocationSuffix()
.toString()).isEqualTo("reactor.core.publisher.Flux.filter");
}

@Test
void stackLineViewPublisherPackagePrefixGetsRemoved() {
String stackLine = "\treactor.core.publisher.Flux.filter(Flux.java:4209)\n";

Traces.StackLineView view =
new Traces.StackLineView(stackLine, 0, stackLine.length()).trim();

assertThat(view.withoutPublisherPackagePrefix()
.toString()).isEqualTo("Flux.filter(Flux.java:4209)");
}
}

0 comments on commit 18379f3

Please sign in to comment.