Skip to content

Using custom data structure for callstack tracking #2630

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.

Sign up for GitHub

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

Merged
bbakerman merged 1 commit into from
Dec 6, 2021
Merged

Using custom data structure for callstack tracking #2630

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
68 changes: 31 additions & 37 deletions src/main/java/graphql/execution/instrumentation/dataloader/FieldLevelTrackingApproach.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,8 @@
import org.dataloader.DataLoaderRegistry;
import org.slf4j.Logger;

import java.util.LinkedHashMap;
import java.util.LinkedHashSet;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.Set;
import java.util.concurrent.CompletableFuture;
import java.util.function.Supplier;
Expand All @@ -32,63 +29,60 @@ public class FieldLevelTrackingApproach {

private static class CallStack implements InstrumentationState {

private final Map<Integer, Integer> expectedFetchCountPerLevel = new LinkedHashMap<>();
private final Map<Integer, Integer> fetchCountPerLevel = new LinkedHashMap<>();
private final Map<Integer, Integer> expectedStrategyCallsPerLevel = new LinkedHashMap<>();
private final Map<Integer, Integer> happenedStrategyCallsPerLevel = new LinkedHashMap<>();
private final Map<Integer, Integer> happenedOnFieldValueCallsPerLevel = new LinkedHashMap<>();

private final IntMap expectedFetchCountPerLevel = new IntMap();
private final IntMap fetchCountPerLevel = new IntMap();
private final IntMap expectedStrategyCallsPerLevel = new IntMap();
private final IntMap happenedStrategyCallsPerLevel = new IntMap();
private final IntMap happenedOnFieldValueCallsPerLevel = new IntMap();

private final Set<Integer> dispatchedLevels = new LinkedHashSet<>();

CallStack() {
expectedStrategyCallsPerLevel.put(1, 1);
expectedStrategyCallsPerLevel.increment(1, 1);
Copy link
Member

@bbakerman bbakerman Dec 5, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you should make another method that is a "set". The above code is "set level 1 to the value 1" whereas your code is "increment" the value by 1 (but its zero right now)

So I think a expectedStrategyCallsPerLevel.set(1) is a better method

}


int increaseExpectedFetchCount(int level, int count) {
expectedFetchCountPerLevel.put(level, expectedFetchCountPerLevel.getOrDefault(level, 0) + count);
return expectedFetchCountPerLevel.get(level);
void increaseExpectedFetchCount(int level, int count) {
expectedFetchCountPerLevel.increment(level, count);
}

void increaseFetchCount(int level) {
fetchCountPerLevel.put(level, fetchCountPerLevel.getOrDefault(level, 0) + 1);
fetchCountPerLevel.increment(level, 1);
}

void increaseExpectedStrategyCalls(int level, int count) {
expectedStrategyCallsPerLevel.put(level, expectedStrategyCallsPerLevel.getOrDefault(level, 0) + count);
expectedStrategyCallsPerLevel.increment(level, count);
}

void increaseHappenedStrategyCalls(int level) {
happenedStrategyCallsPerLevel.put(level, happenedStrategyCallsPerLevel.getOrDefault(level, 0) + 1);
happenedStrategyCallsPerLevel.increment(level, 1);
}

void increaseHappenedOnFieldValueCalls(int level) {
happenedOnFieldValueCallsPerLevel.put(level, happenedOnFieldValueCallsPerLevel.getOrDefault(level, 0) + 1);
happenedOnFieldValueCallsPerLevel.increment(level, 1);
}

boolean allStrategyCallsHappened(int level) {
return Objects.equals(happenedStrategyCallsPerLevel.get(level), expectedStrategyCallsPerLevel.get(level));
return happenedStrategyCallsPerLevel.get(level) == expectedStrategyCallsPerLevel.get(level);
}

boolean allOnFieldCallsHappened(int level) {
return Objects.equals(happenedOnFieldValueCallsPerLevel.get(level), expectedStrategyCallsPerLevel.get(level));
return happenedOnFieldValueCallsPerLevel.get(level) == expectedStrategyCallsPerLevel.get(level);
}

boolean allFetchesHappened(int level) {
return Objects.equals(fetchCountPerLevel.get(level), expectedFetchCountPerLevel.get(level));
return fetchCountPerLevel.get(level) == expectedFetchCountPerLevel.get(level);
}

@Override
public String toString() {
return "CallStack{" +
"expectedFetchCountPerLevel=" + expectedFetchCountPerLevel +
", fetchCountPerLevel=" + fetchCountPerLevel +
", expectedStrategyCallsPerLevel=" + expectedStrategyCallsPerLevel +
", happenedStrategyCallsPerLevel=" + happenedStrategyCallsPerLevel +
", happenedOnFieldValueCallsPerLevel=" + happenedOnFieldValueCallsPerLevel +
", dispatchedLevels" + dispatchedLevels +
'}';
"expectedFetchCountPerLevel=" + expectedFetchCountPerLevel +
", fetchCountPerLevel=" + fetchCountPerLevel +
", expectedStrategyCallsPerLevel=" + expectedStrategyCallsPerLevel +
", happenedStrategyCallsPerLevel=" + happenedStrategyCallsPerLevel +
", happenedOnFieldValueCallsPerLevel=" + happenedOnFieldValueCallsPerLevel +
", dispatchedLevels" + dispatchedLevels +
'}';
}

public boolean dispatchIfNotDispatchedBefore(int level) {
Expand All @@ -101,17 +95,17 @@ public boolean dispatchIfNotDispatchedBefore(int level) {
}

public void clearAndMarkCurrentLevelAsReady(int level) {
expectedFetchCountPerLevel.clear();
fetchCountPerLevel.clear();
expectedStrategyCallsPerLevel.clear();
happenedStrategyCallsPerLevel.clear();
happenedOnFieldValueCallsPerLevel.clear();
expectedFetchCountPerLevel.reset();
fetchCountPerLevel.reset();
expectedStrategyCallsPerLevel.reset();
happenedStrategyCallsPerLevel.reset();
happenedOnFieldValueCallsPerLevel.reset();
dispatchedLevels.clear();

// make sure the level is ready
expectedFetchCountPerLevel.put(level, 1);
expectedStrategyCallsPerLevel.put(level, 1);
happenedStrategyCallsPerLevel.put(level, 1);
expectedFetchCountPerLevel.increment(level, 1);
expectedStrategyCallsPerLevel.increment(level, 1);
happenedStrategyCallsPerLevel.increment(level, 1);
}
}

Expand Down Expand Up @@ -241,7 +235,7 @@ private boolean levelReady(CallStack callStack, int level) {
return callStack.allFetchesHappened(1);
}
if (levelReady(callStack, level - 1) && callStack.allOnFieldCallsHappened(level - 1)
&& callStack.allStrategyCallsHappened(level) && callStack.allFetchesHappened(level)) {
&& callStack.allStrategyCallsHappened(level) && callStack.allFetchesHappened(level)) {
return true;
}
return false;
Expand Down
62 changes: 62 additions & 0 deletions src/main/java/graphql/execution/instrumentation/dataloader/IntMap.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
package graphql.execution.instrumentation.dataloader;

import graphql.Internal;

import java.util.Arrays;

@Internal
public class IntMap {

// A reasonable default that guarantees no additional allocations for most use cases.
private static final int DEFAULT_INITIAL_SIZE = 16;

// this array is mutable in both size and contents.
private int[] countsByLevel;

public IntMap(int initialSize) {
if (initialSize < 0) {
throw new IllegalArgumentException("negative size " + initialSize);
}
countsByLevel = new int[initialSize];
}

public IntMap() {
this(DEFAULT_INITIAL_SIZE);
}

public int get(int level) {
if (level < 0) {
throw new IllegalArgumentException("negative level " + level);
}
if (level + 1 > countsByLevel.length) {
throw new IllegalArgumentException("unknown level " + level);
}
return countsByLevel[level];
}

public void increment(int level, int by) {
if (level < 0) {
throw new IllegalArgumentException("negative level " + level);
}
if (level + 1 > countsByLevel.length) {
int newSize = level == 0 ? 1 : level * 2;
countsByLevel = Arrays.copyOf(countsByLevel, newSize);
}
countsByLevel[level] += by;
}

@Override
public String toString() {
StringBuilder result = new StringBuilder();
result.append("IntMap[");
for (int i = 0; i < countsByLevel.length; i++) {
result.append("level=").append(i).append(",count=").append(countsByLevel[i]).append(" ");
}
result.append("]");
return result.toString();
}

public void reset() {
Arrays.fill(countsByLevel, 0);
}
}
80 changes: 80 additions & 0 deletions src/test/groovy/graphql/execution/instrumentation/dataloader/IntMapTest.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
package graphql.execution.instrumentation.dataloader

import spock.lang.Specification

class IntMapTest extends Specification {

def "increase adds levels"() {
given:
IntMap sut = new IntMap(0) // starts empty

when:
sut.increment(2, 42) // level 2 has count 42

then:
sut.get(0) == 0
sut.get(1) == 0
sut.get(2) == 42
}

def "increase count by 10 for every level"() {
given:
IntMap sut = new IntMap(0)

when:
5.times {Integer level ->
sut.increment(level, 10)
}

then:
5.times { Integer level ->
sut.get(level) == 10
}
}

def "increase yields new count"() {
given:
IntMap sut = new IntMap(0)

when:
sut.increment(1, 0)

then:
sut.get(1) == 0

when:
sut.increment(1, 1)

then:
sut.get(1) == 1

when:
sut.increment(1, 100)

then:
sut.get(1) == 101
}

def "toString() is important for debugging"() {
given:
IntMap sut = new IntMap(0)

when:
sut.toString()

then:
sut.toString() == "IntMap[]"

when:
sut.increment(0, 42)

then:
sut.toString() == "IntMap[level=0,count=42 ]"

when:
sut.increment(1, 1)

then:
sut.toString() == "IntMap[level=0,count=42 level=1,count=1 ]"
}
}
45 changes: 45 additions & 0 deletions src/test/java/benchmark/IntMapBenchmark.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
package benchmark;

import graphql.execution.instrumentation.dataloader.IntMap;
import org.openjdk.jmh.annotations.Benchmark;
import org.openjdk.jmh.annotations.BenchmarkMode;
import org.openjdk.jmh.annotations.Measurement;
import org.openjdk.jmh.annotations.Mode;
import org.openjdk.jmh.annotations.Scope;
import org.openjdk.jmh.annotations.State;
import org.openjdk.jmh.annotations.Warmup;
import org.openjdk.jmh.infra.Blackhole;

import java.util.LinkedHashMap;
import java.util.Map;
import java.util.concurrent.TimeUnit;

@State(Scope.Benchmark)
@BenchmarkMode(Mode.Throughput)
@Warmup(iterations = 2)
@Measurement(iterations = 2, timeUnit = TimeUnit.NANOSECONDS)
public class IntMapBenchmark {

@Benchmark
public void benchmarkLinkedHashMap(Blackhole blackhole) {
Map<Integer, Integer> result = new LinkedHashMap<>();
for (int i = 0; i < 30; i++) {
int level = i % 10;
int count = i * 2;
result.put(level, result.getOrDefault(level, 0) + count);
blackhole.consume(result.get(level));
}
}

@Benchmark
public void benchmarkIntMap(Blackhole blackhole) {
IntMap result = new IntMap(16);
for (int i = 0; i < 30; i++) {
int level = i % 10;
int count = i * 2;
result.increment(level, count);
blackhole.consume(result.get(level));
}
}
}