Skip to content

Commit

Permalink
[Profiling] Use plain arrays in stack traces
Browse files Browse the repository at this point in the history 
With this commit we refactor the internal representation of stacktraces
to use plain arrays instead of lists for some of its properties. The
motivation behind this change is simplicity:

* It avoids unnecessary boxing
* We could eliminate a few redundant null checks because we use
  primitive types now in some places
* We could slightly simplify runlength decoding
  • Loading branch information
@danielmitterdorfer
danielmitterdorfer committed Feb 7, 2024
1 parent 59946ed commit afed275
Show file tree
Hide file tree
Showing 7 changed files with 86 additions and 93 deletions.
24 changes: 12 additions & 12 deletions ...rc/internalClusterTest/java/org/elasticsearch/xpack/profiling/GetStackTracesActionIT.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,10 +29,10 @@ public void testGetStackTracesUnfiltered() throws Exception {
assertNotNull(response.getStackTraces());
// just do a high-level spot check. Decoding is tested in unit-tests
StackTrace stackTrace = response.getStackTraces().get("L7kj7UvlKbT-vN73el4faQ");
assertEquals(18, stackTrace.addressOrLines.size());
assertEquals(18, stackTrace.fileIds.size());
assertEquals(18, stackTrace.frameIds.size());
assertEquals(18, stackTrace.typeIds.size());
assertEquals(18, stackTrace.addressOrLines.length);
assertEquals(18, stackTrace.fileIds.length);
assertEquals(18, stackTrace.frameIds.length);
assertEquals(18, stackTrace.typeIds.length);
assertEquals(0.0000048475146d, stackTrace.annualCO2Tons, 0.0000000001d);
assertEquals(0.18834d, stackTrace.annualCostsUSD, 0.00001d);

Expand Down Expand Up @@ -73,10 +73,10 @@ public void testGetStackTracesFromAPMWithMatchNoDownsampling() throws Exception
assertNotNull(response.getStackTraces());
// just do a high-level spot check. Decoding is tested in unit-tests
StackTrace stackTrace = response.getStackTraces().get("Ce77w10WeIDow3kd1jowlA");
assertEquals(39, stackTrace.addressOrLines.size());
assertEquals(39, stackTrace.fileIds.size());
assertEquals(39, stackTrace.frameIds.size());
assertEquals(39, stackTrace.typeIds.size());
assertEquals(39, stackTrace.addressOrLines.length);
assertEquals(39, stackTrace.fileIds.length);
assertEquals(39, stackTrace.frameIds.length);
assertEquals(39, stackTrace.typeIds.length);
assertTrue(stackTrace.annualCO2Tons > 0.0d);
assertTrue(stackTrace.annualCostsUSD > 0.0d);

Expand Down Expand Up @@ -139,10 +139,10 @@ public int hashCode() {
assertNotNull(response.getStackTraces());
// just do a high-level spot check. Decoding is tested in unit-tests
StackTrace stackTrace = response.getStackTraces().get("Ce77w10WeIDow3kd1jowlA");
assertEquals(39, stackTrace.addressOrLines.size());
assertEquals(39, stackTrace.fileIds.size());
assertEquals(39, stackTrace.frameIds.size());
assertEquals(39, stackTrace.typeIds.size());
assertEquals(39, stackTrace.addressOrLines.length);
assertEquals(39, stackTrace.fileIds.length);
assertEquals(39, stackTrace.frameIds.length);
assertEquals(39, stackTrace.typeIds.length);
assertTrue(stackTrace.annualCO2Tons > 0.0d);
assertTrue(stackTrace.annualCostsUSD > 0.0d);

Expand Down
65 changes: 31 additions & 34 deletions x-pack/plugin/profiling/src/main/java/org/elasticsearch/xpack/profiling/StackTrace.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,7 @@
import org.elasticsearch.xcontent.XContentBuilder;

import java.io.IOException;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.function.Consumer;
Expand All @@ -25,20 +23,20 @@ final class StackTrace implements ToXContentObject {

static final int NATIVE_FRAME_TYPE = 3;
static final int KERNEL_FRAME_TYPE = 4;
List<Integer> addressOrLines;
List<String> fileIds;
List<String> frameIds;
List<Integer> typeIds;
int[] addressOrLines;
String[] fileIds;
String[] frameIds;
int[] typeIds;

double annualCO2Tons;
double annualCostsUSD;
long count;

StackTrace(
List<Integer> addressOrLines,
List<String> fileIds,
List<String> frameIds,
List<Integer> typeIds,
int[] addressOrLines,
String[] fileIds,
String[] frameIds,
int[] typeIds,
double annualCO2Tons,
double annualCostsUSD,
long count
Expand Down Expand Up @@ -84,8 +82,8 @@ final class StackTrace implements ToXContentObject {
* @return Corresponding numbers that are encoded in the input.
*/
// package-private for testing
static List<Integer> runLengthDecodeBase64Url(String input, int size, int capacity) {
Integer[] output = new Integer[capacity];
static int[] runLengthDecodeBase64Url(String input, int size, int capacity) {
int[] output = new int[capacity];
int multipleOf8 = size / 8;
int remainder = size % 8;

Expand Down Expand Up @@ -138,7 +136,6 @@ static List<Integer> runLengthDecodeBase64Url(String input, int size, int capaci
value = n & 0xff;

Arrays.fill(output, j, j + count, value);
j += count;
} else if (remainder == 3) {
n = (charCodeAt(input, i) << 12) | (charCodeAt(input, i + 1) << 6) | charCodeAt(input, i + 2);
n >>= 2;
Expand All @@ -147,12 +144,8 @@ static List<Integer> runLengthDecodeBase64Url(String input, int size, int capaci
value = n & 0xff;

Arrays.fill(output, j, j + count, value);
j += count;
}
if (j < capacity) {
Arrays.fill(output, j, capacity, 0);
}
return Arrays.asList(output);
return output;
}

// package-private for testing
Expand Down Expand Up @@ -195,9 +188,9 @@ public static StackTrace fromSource(Map<String, Object> source) {
String inputFrameTypes = ObjectPath.eval(PATH_FRAME_TYPES, source);
int countsFrameIDs = inputFrameIDs.length() / BASE64_FRAME_ID_LENGTH;

List<String> fileIDs = new ArrayList<>(countsFrameIDs);
List<String> frameIDs = new ArrayList<>(countsFrameIDs);
List<Integer> addressOrLines = new ArrayList<>(countsFrameIDs);
String[] fileIDs = new String[countsFrameIDs];
String[] frameIDs = new String[countsFrameIDs];
int[] addressOrLines = new int[countsFrameIDs];

// Step 1: Convert the base64-encoded frameID list into two separate
// lists (frame IDs and file IDs), both of which are also base64-encoded.
Expand All @@ -210,22 +203,22 @@ public static StackTrace fromSource(Map<String, Object> source) {
// address (see diagram in definition of EncodedStackTrace).
for (int i = 0, pos = 0; i < countsFrameIDs; i++, pos += BASE64_FRAME_ID_LENGTH) {
String frameID = inputFrameIDs.substring(pos, pos + BASE64_FRAME_ID_LENGTH);
frameIDs.add(frameID);
fileIDs.add(getFileIDFromStackFrameID(frameID));
addressOrLines.add(getAddressFromStackFrameID(frameID));
frameIDs[i] = frameID;
fileIDs[i] = getFileIDFromStackFrameID(frameID);
addressOrLines[i] = getAddressFromStackFrameID(frameID);
}

// Step 2: Convert the run-length byte encoding into a list of uint8s.
List<Integer> typeIDs = runLengthDecodeBase64Url(inputFrameTypes, inputFrameTypes.length(), countsFrameIDs);
int[] typeIDs = runLengthDecodeBase64Url(inputFrameTypes, inputFrameTypes.length(), countsFrameIDs);

return new StackTrace(addressOrLines, fileIDs, frameIDs, typeIDs, 0, 0, 0);
}

public void forNativeAndKernelFrames(Consumer<String> consumer) {
for (int i = 0; i < this.fileIds.size(); i++) {
Integer frameType = this.typeIds.get(i);
if (frameType != null && (frameType == NATIVE_FRAME_TYPE || frameType == KERNEL_FRAME_TYPE)) {
consumer.accept(this.fileIds.get(i));
for (int i = 0; i < this.fileIds.length; i++) {
int frameType = this.typeIds[i];
if (frameType == NATIVE_FRAME_TYPE || frameType == KERNEL_FRAME_TYPE) {
consumer.accept(this.fileIds[i]);
}
}
}
Expand All @@ -251,16 +244,20 @@ public boolean equals(Object o) {
return false;
}
StackTrace that = (StackTrace) o;
return addressOrLines.equals(that.addressOrLines)
&& fileIds.equals(that.fileIds)
&& frameIds.equals(that.frameIds)
&& typeIds.equals(that.typeIds);
return Arrays.equals(addressOrLines, that.addressOrLines)
&& Arrays.equals(fileIds, that.fileIds)
&& Arrays.equals(frameIds, that.frameIds)
&& Arrays.equals(typeIds, that.typeIds);
// Don't compare metadata like annualized co2, annualized costs and count.
}

// Don't hash metadata like annualized co2, annualized costs and count.
@Override
public int hashCode() {
return Objects.hash(addressOrLines, fileIds, frameIds, typeIds);
int result = Arrays.hashCode(addressOrLines);
result = 31 * result + Arrays.hashCode(fileIds);
result = 31 * result + Arrays.hashCode(frameIds);
result = 31 * result + Arrays.hashCode(typeIds);
return result;
}
}
12 changes: 6 additions & 6 deletions ...ofiling/src/main/java/org/elasticsearch/xpack/profiling/TransportGetFlamegraphAction.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -91,12 +91,12 @@ static GetFlamegraphResponse buildFlamegraph(GetStackTracesResponse response) {
builder.addAnnualCostsUSDInclusive(0, annualCostsUSD);
builder.addAnnualCostsUSDExclusive(0, 0.0d);

int frameCount = stackTrace.frameIds.size();
int frameCount = stackTrace.frameIds.length;
for (int i = 0; i < frameCount; i++) {
String frameId = stackTrace.frameIds.get(i);
String fileId = stackTrace.fileIds.get(i);
Integer frameType = stackTrace.typeIds.get(i);
Integer addressOrLine = stackTrace.addressOrLines.get(i);
String frameId = stackTrace.frameIds[i];
String fileId = stackTrace.fileIds[i];
int frameType = stackTrace.typeIds[i];
int addressOrLine = stackTrace.addressOrLines[i];
StackFrame stackFrame = response.getStackFrames().getOrDefault(frameId, EMPTY_STACKFRAME);
String executable = response.getExecutables().getOrDefault(fileId, "");
final boolean isLeafFrame = i == frameCount - 1;
Expand Down Expand Up @@ -199,7 +199,7 @@ public int addNode(
int frameType,
boolean inline,
String fileName,
Integer addressOrLine,
int addressOrLine,
String functionName,
int functionOffset,
String sourceFileName,
Expand Down
4 changes: 2 additions & 2 deletions ...filing/src/main/java/org/elasticsearch/xpack/profiling/TransportGetStackTracesAction.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -567,8 +567,8 @@ public void onStackTraceResponse(MultiGetResponse multiGetItemResponses) {
StackTrace stacktrace = StackTrace.fromSource(trace.getResponse().getSource());
// Guard against concurrent access and ensure we only handle each item once
if (stackTracePerId.putIfAbsent(id, stacktrace) == null) {
totalFrames.addAndGet(stacktrace.frameIds.size());
stackFrameIds.addAll(stacktrace.frameIds);
totalFrames.addAndGet(stacktrace.frameIds.length);
stackFrameIds.addAll(List.of(stacktrace.frameIds));
stacktrace.forNativeAndKernelFrames(e -> executableIds.add(e));
}
}
Expand Down
9 changes: 4 additions & 5 deletions ...rofiling/src/test/java/org/elasticsearch/xpack/profiling/GetStackTracesResponseTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
import org.elasticsearch.test.AbstractChunkedSerializingTestCase;
import org.elasticsearch.test.ESTestCase;

import java.util.List;
import java.util.Map;

public class GetStackTracesResponseTests extends ESTestCase {
Expand All @@ -25,10 +24,10 @@ private GetStackTracesResponse createTestInstance() {
Map.of(
"QjoLteG7HX3VUUXr-J4kHQ",
new StackTrace(
List.of(1083999),
List.of("QCCDqjSg3bMK1C4YRK6Tiw"),
List.of("QCCDqjSg3bMK1C4YRK6TiwAAAAAAEIpf"),
List.of(2),
new int[] { 1083999 },
new String[] { "QCCDqjSg3bMK1C4YRK6Tiw" },
new String[] { "QCCDqjSg3bMK1C4YRK6TiwAAAAAAEIpf" },
new int[] { 2 },
0.3d,
2.7d,
1
Expand Down
51 changes: 25 additions & 26 deletions x-pack/plugin/profiling/src/test/java/org/elasticsearch/xpack/profiling/StackTraceTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,7 @@
import org.elasticsearch.xcontent.XContentType;

import java.io.IOException;
import java.util.ArrayList;
import java.util.List;
import java.util.Arrays;
import java.util.Map;

import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertToXContentEquivalent;
Expand All @@ -33,30 +32,30 @@ public void testDecodeFrameId() {
public void testRunlengthDecodeUniqueValues() {
// 0 - 9 (reversed)
String encodedFrameTypes = "AQkBCAEHAQYBBQEEAQMBAgEBAQA";
List<Integer> actual = StackTrace.runLengthDecodeBase64Url(encodedFrameTypes, encodedFrameTypes.length(), 10);
assertEquals(List.of(9, 8, 7, 6, 5, 4, 3, 2, 1, 0), actual);
int[] actual = StackTrace.runLengthDecodeBase64Url(encodedFrameTypes, encodedFrameTypes.length(), 10);
assertArrayEquals(new int[] { 9, 8, 7, 6, 5, 4, 3, 2, 1, 0 }, actual);
}

public void testRunlengthDecodeSingleValue() {
// "4", repeated ten times
String encodedFrameTypes = "CgQ";
List<Integer> actual = StackTrace.runLengthDecodeBase64Url(encodedFrameTypes, encodedFrameTypes.length(), 10);
assertEquals(List.of(4, 4, 4, 4, 4, 4, 4, 4, 4, 4), actual);
int[] actual = StackTrace.runLengthDecodeBase64Url(encodedFrameTypes, encodedFrameTypes.length(), 10);
assertArrayEquals(new int[] { 4, 4, 4, 4, 4, 4, 4, 4, 4, 4 }, actual);
}

public void testRunlengthDecodeFillsGap() {
// "2", repeated three times
String encodedFrameTypes = "AwI";
List<Integer> actual = StackTrace.runLengthDecodeBase64Url(encodedFrameTypes, encodedFrameTypes.length(), 5);
int[] actual = StackTrace.runLengthDecodeBase64Url(encodedFrameTypes, encodedFrameTypes.length(), 5);
// zeroes should be appended for the last two values which are not present in the encoded representation.
assertEquals(List.of(2, 2, 2, 0, 0), actual);
assertArrayEquals(new int[] { 2, 2, 2, 0, 0 }, actual);
}

public void testRunlengthDecodeMixedValue() {
// 4
String encodedFrameTypes = "BQADAg";
List<Integer> actual = StackTrace.runLengthDecodeBase64Url(encodedFrameTypes, encodedFrameTypes.length(), 8);
assertEquals(List.of(0, 0, 0, 0, 0, 2, 2, 2), actual);
int[] actual = StackTrace.runLengthDecodeBase64Url(encodedFrameTypes, encodedFrameTypes.length(), 8);
assertArrayEquals(new int[] { 0, 0, 0, 0, 0, 2, 2, 2 }, actual);
}

public void testCreateFromSource() {
Expand All @@ -73,10 +72,10 @@ public void testCreateFromSource() {
)
);
// end::noformat
assertEquals(List.of("AAAAAAAAAAUAAAAAAAAB3gAAAAAAD67u"), stackTrace.frameIds);
assertEquals(List.of("AAAAAAAAAAUAAAAAAAAB3g"), stackTrace.fileIds);
assertEquals(List.of(1027822), stackTrace.addressOrLines);
assertEquals(List.of(2), stackTrace.typeIds);
assertArrayEquals(new String[] { "AAAAAAAAAAUAAAAAAAAB3gAAAAAAD67u" }, stackTrace.frameIds);
assertArrayEquals(new String[] { "AAAAAAAAAAUAAAAAAAAB3g" }, stackTrace.fileIds);
assertArrayEquals(new int[] { 1027822 }, stackTrace.addressOrLines);
assertArrayEquals(new int[] { 2 }, stackTrace.typeIds);
}

public void testToXContent() throws IOException {
Expand All @@ -94,10 +93,10 @@ public void testToXContent() throws IOException {

XContentBuilder actualRequest = XContentFactory.contentBuilder(contentType);
StackTrace stackTrace = new StackTrace(
List.of(1027822),
List.of("AAAAAAAAAAUAAAAAAAAB3g"),
List.of("AAAAAAAAAAUAAAAAAAAB3gAAAAAAD67u"),
List.of(2),
new int[] { 1027822 },
new String[] { "AAAAAAAAAAUAAAAAAAAB3g" },
new String[] { "AAAAAAAAAAUAAAAAAAAB3gAAAAAAD67u" },
new int[] { 2 },
0.3d,
2.7d,
1
Expand All @@ -109,10 +108,10 @@ public void testToXContent() throws IOException {

public void testEquality() {
StackTrace stackTrace = new StackTrace(
List.of(102782),
List.of("AAAAAAAAAAUAAAAAAAAB3g"),
List.of("AAAAAAAAAAUAAAAAAAAB3gAAAAAAD67u"),
List.of(2),
new int[] { 102782 },
new String[] { "AAAAAAAAAAUAAAAAAAAB3g" },
new String[] { "AAAAAAAAAAUAAAAAAAAB3gAAAAAAD67u" },
new int[] { 2 },
0.3d,
2.7d,
1
Expand All @@ -121,10 +120,10 @@ public void testEquality() {
EqualsHashCodeTestUtils.checkEqualsAndHashCode(
stackTrace,
(o -> new StackTrace(
new ArrayList<>(o.addressOrLines),
new ArrayList<>(o.fileIds),
new ArrayList<>(o.frameIds),
new ArrayList<>(o.typeIds),
Arrays.copyOf(o.addressOrLines, o.addressOrLines.length),
Arrays.copyOf(o.fileIds, o.fileIds.length),
Arrays.copyOf(o.frameIds, o.frameIds.length),
Arrays.copyOf(o.typeIds, o.typeIds.length),
0.3d,
2.7d,
1
Expand Down
14 changes: 6 additions & 8 deletions ...ng/src/test/java/org/elasticsearch/xpack/profiling/TransportGetFlamegraphActionTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,8 @@ public void testCreateFlamegraph() {
Map.of(
"2buqP1GpF-TXYmL4USW8gA",
new StackTrace(
List.of(12784352, 19334053, 19336161, 18795859, 18622708, 18619213, 12989721, 13658842, 16339645),
List.of(
new int[] { 12784352, 19334053, 19336161, 18795859, 18622708, 18619213, 12989721, 13658842, 16339645 },
new String[] {
"fr28zxcZ2UDasxYuu6dV-w",
"fr28zxcZ2UDasxYuu6dV-w",
"fr28zxcZ2UDasxYuu6dV-w",
Expand All @@ -28,9 +28,8 @@ public void testCreateFlamegraph() {
"fr28zxcZ2UDasxYuu6dV-w",
"fr28zxcZ2UDasxYuu6dV-w",
"fr28zxcZ2UDasxYuu6dV-w",
"fr28zxcZ2UDasxYuu6dV-w"
),
List.of(
"fr28zxcZ2UDasxYuu6dV-w" },
new String[] {
"fr28zxcZ2UDasxYuu6dV-wAAAAAAwxLg",
"fr28zxcZ2UDasxYuu6dV-wAAAAABJwOl",
"fr28zxcZ2UDasxYuu6dV-wAAAAABJwvh",
Expand All @@ -39,9 +38,8 @@ public void testCreateFlamegraph() {
"fr28zxcZ2UDasxYuu6dV-wAAAAABHBtN",
"fr28zxcZ2UDasxYuu6dV-wAAAAAAxjUZ",
"fr28zxcZ2UDasxYuu6dV-wAAAAAA0Gra",
"fr28zxcZ2UDasxYuu6dV-wAAAAAA-VK9"
),
List.of(3, 3, 3, 3, 3, 3, 3, 3, 3),
"fr28zxcZ2UDasxYuu6dV-wAAAAAA-VK9" },
new int[] { 3, 3, 3, 3, 3, 3, 3, 3, 3 },
0.3d,
2.7d,
1
Expand Down

0 comments on commit afed275

Please sign in to comment.