Skip to content

Commit 05d60ad

Browse files
andimarekandimarek
authored
Merge pull request #241 from graphql-java/no-locks
remove any Reentrant locks in favor of a CAS linked list
2 parents 77bac05 + 54197a0 commit 05d60ad

File tree

13 files changed

+366
-166
lines changed

13 files changed

+366
-166
lines changed

‎.github/workflows/master.yml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,6 @@ jobs:
2828
- name: Setup Gradle
2929
uses: gradle/actions/setup-gradle@v5
3030
- name: build test and publish
31-
run: ./gradlew assemble && ./gradlew check --info && ./gradlew publishToSonatype closeAndReleaseSonatypeStagingRepository -x check --info --stacktrace
31+
run: ./gradlew assemble && ./gradlew check --info && && ./gradlew jcstress && ./gradlew publishToSonatype closeAndReleaseSonatypeStagingRepository -x check --info --stacktrace
3232
env:
3333
CI: true

‎.github/workflows/pull_request.yml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,6 @@ jobs:
2626
- name: Setup Gradle
2727
uses: gradle/actions/setup-gradle@v5
2828
- name: build and test
29-
run: ./gradlew assemble && ./gradlew check --info --stacktrace
29+
run: ./gradlew assemble && ./gradlew check --info --stacktrace && ./gradlew jcstress
3030
env:
3131
CI: true

‎build.gradle‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
1+
import net.ltgt.gradle.errorprone.CheckSeverity
12
import org.jetbrains.kotlin.gradle.dsl.JvmTarget
23
import org.jetbrains.kotlin.gradle.dsl.KotlinVersion
3-
import net.ltgt.gradle.errorprone.CheckSeverity
4+
45
import java.text.SimpleDateFormat
56

67
plugins {
@@ -15,6 +16,7 @@ plugins {
1516
id 'com.github.ben-manes.versions' version '0.53.0'
1617
id "me.champeau.jmh" version "0.7.3"
1718
id "net.ltgt.errorprone" version '4.3.0'
19+
id "io.github.reyerizo.gradle.jcstress" version "0.8.15"
1820

1921
// Kotlin just for tests - not production code
2022
id 'org.jetbrains.kotlin.jvm' version '2.2.20'
@@ -229,7 +231,8 @@ nexusPublishing {
229231
// https://central.sonatype.org/publish/publish-portal-ossrh-staging-api/#configuration
230232
nexusUrl.set(uri("https://ossrh-staging-api.central.sonatype.com/service/local/"))
231233
// GraphQL Java does not publish snapshots, but adding this URL for completeness
232-
snapshotRepositoryUrl.set(uri("https://central.sonatype.com/repository/maven-snapshots/")) }
234+
snapshotRepositoryUrl.set(uri("https://central.sonatype.com/repository/maven-snapshots/"))
235+
}
233236
}
234237
}
235238

@@ -258,3 +261,7 @@ tasks.named("dependencyUpdates").configure {
258261
isNonStable(it.candidate.version)
259262
}
260263
}
264+
265+
jcstress {
266+
// verbose = true
267+
}

‎src/jcstress/java/org/dataloader/DataLoader_Batching_Caching_JCStress.java‎

Lines changed: 108 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,108 @@
1+
package org.dataloader;
2+
3+
import org.openjdk.jcstress.annotations.Actor;
4+
import org.openjdk.jcstress.annotations.Arbiter;
5+
import org.openjdk.jcstress.annotations.JCStressTest;
6+
import org.openjdk.jcstress.annotations.Outcome;
7+
import org.openjdk.jcstress.annotations.State;
8+
import org.openjdk.jcstress.infra.results.II_Result;
9+
10+
import java.util.List;
11+
import java.util.concurrent.CompletableFuture;
12+
import java.util.concurrent.atomic.AtomicInteger;
13+
14+
import static org.openjdk.jcstress.annotations.Expect.ACCEPTABLE;
15+
16+
@JCStressTest
17+
@State
18+
@Outcome(id = "2000, 2000", expect = ACCEPTABLE, desc = "accepted")
19+
public class DataLoader_Batching_Caching_JCStress {
20+
21+
22+
AtomicInteger counter = new AtomicInteger();
23+
AtomicInteger batchLoaderCount = new AtomicInteger();
24+
volatile boolean finished1;
25+
volatile boolean finished2;
26+
27+
28+
BatchLoader<String, String> batchLoader = keys -> {
29+
return CompletableFuture.supplyAsync(() -> {
30+
batchLoaderCount.getAndAdd(keys.size());
31+
return keys;
32+
});
33+
};
34+
DataLoader<String, String> dataLoader = DataLoaderFactory.newDataLoader(batchLoader);
35+
36+
public DataLoader_Batching_Caching_JCStress() {
37+
38+
}
39+
40+
@Actor
41+
public void load1() {
42+
for (int i = 0; i < 1000; i++) {
43+
dataLoader.load("load-1-" + i);
44+
}
45+
// we load the same keys again
46+
for (int i = 0; i < 1000; i++) {
47+
dataLoader.load("load-1-" + i);
48+
}
49+
finished1 = true;
50+
}
51+
52+
@Actor
53+
public void load2() {
54+
for (int i = 0; i < 1000; i++) {
55+
dataLoader.load("load-2-" + i);
56+
}
57+
// we load the same keys again
58+
for (int i = 0; i < 1000; i++) {
59+
dataLoader.load("load-2-" + i);
60+
}
61+
finished2 = true;
62+
}
63+
64+
65+
@Actor
66+
public void dispatch1() {
67+
while (!finished1 || !finished2) {
68+
try {
69+
List<String> dispatchedResult = dataLoader.dispatch().get();
70+
counter.getAndAdd(dispatchedResult.size());
71+
} catch (Exception e) {
72+
throw new RuntimeException(e);
73+
}
74+
}
75+
try {
76+
List<String> dispatchedResult = dataLoader.dispatch().get();
77+
counter.getAndAdd(dispatchedResult.size());
78+
} catch (Exception e) {
79+
throw new RuntimeException(e);
80+
}
81+
}
82+
83+
@Actor
84+
public void dispatch2() {
85+
while (!finished1 || !finished2) {
86+
try {
87+
List<String> dispatchedResult = dataLoader.dispatch().get();
88+
counter.getAndAdd(dispatchedResult.size());
89+
} catch (Exception e) {
90+
throw new RuntimeException(e);
91+
}
92+
}
93+
try {
94+
List<String> dispatchedResult = dataLoader.dispatch().get();
95+
counter.getAndAdd(dispatchedResult.size());
96+
} catch (Exception e) {
97+
throw new RuntimeException(e);
98+
}
99+
}
100+
101+
@Arbiter
102+
public void arbiter(II_Result r) {
103+
r.r1 = counter.get();
104+
r.r2 = batchLoaderCount.get();
105+
}
106+
107+
108+
}

‎src/jcstress/java/org/dataloader/DataLoader_NoBatching_Caching_JCStress.java‎

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
package org.dataloader;
2+
3+
import org.openjdk.jcstress.annotations.Actor;
4+
import org.openjdk.jcstress.annotations.Arbiter;
5+
import org.openjdk.jcstress.annotations.JCStressTest;
6+
import org.openjdk.jcstress.annotations.Outcome;
7+
import org.openjdk.jcstress.annotations.State;
8+
import org.openjdk.jcstress.infra.results.II_Result;
9+
10+
import java.util.concurrent.CompletableFuture;
11+
import java.util.concurrent.atomic.AtomicInteger;
12+
13+
import static org.openjdk.jcstress.annotations.Expect.ACCEPTABLE;
14+
import static org.openjdk.jcstress.annotations.Expect.ACCEPTABLE_INTERESTING;
15+
16+
@JCStressTest
17+
@State
18+
@Outcome(id = "1000, 1000", expect = ACCEPTABLE, desc = "No keys loaded twice")
19+
@Outcome(id = "1.*, 1000", expect = ACCEPTABLE_INTERESTING, desc = "Some keys loaded twice")
20+
public class DataLoader_NoBatching_Caching_JCStress {
21+
22+
23+
AtomicInteger batchLoaderCount = new AtomicInteger();
24+
25+
BatchLoader<String, String> batchLoader = keys -> {
26+
batchLoaderCount.getAndAdd(keys.size());
27+
return CompletableFuture.completedFuture(keys);
28+
};
29+
30+
31+
DataLoader<String, String> dataLoader = DataLoaderFactory.newDataLoader(batchLoader, DataLoaderOptions.newOptions().setBatchingEnabled(false).build());
32+
33+
@Actor
34+
public void load1() {
35+
for (int i = 0; i < 1000; i++) {
36+
dataLoader.load("load-1-" + i);
37+
}
38+
}
39+
40+
@Actor
41+
public void load2() {
42+
for (int i = 0; i < 1000; i++) {
43+
dataLoader.load("load-1-" + i);
44+
}
45+
}
46+
47+
48+
@Arbiter
49+
public void arbiter(II_Result r) {
50+
r.r1 = batchLoaderCount.get();
51+
r.r2 = dataLoader.getCacheMap().size();
52+
}
53+
54+
}

‎src/jmh/java/performance/DataLoaderDispatchPerformance.java‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
import org.openjdk.jmh.annotations.Scope;
1313
import org.openjdk.jmh.annotations.Setup;
1414
import org.openjdk.jmh.annotations.State;
15+
import org.openjdk.jmh.annotations.Threads;
1516
import org.openjdk.jmh.annotations.Warmup;
1617
import org.openjdk.jmh.infra.Blackhole;
1718

@@ -280,15 +281,20 @@ public void setup() {
280281

281282
}
282283

284+
DataLoader ownerDL = DataLoaderFactory.newDataLoader(ownerBatchLoader);
285+
DataLoader petDL = DataLoaderFactory.newDataLoader(petBatchLoader);
286+
287+
283288
}
284289

285290

286291
@Benchmark
287292
@BenchmarkMode(Mode.AverageTime)
288293
@OutputTimeUnit(TimeUnit.NANOSECONDS)
294+
@Threads(Threads.MAX)
289295
public void loadAndDispatch(MyState myState, Blackhole blackhole) {
290-
DataLoader ownerDL = DataLoaderFactory.newDataLoader(ownerBatchLoader);
291-
DataLoader petDL = DataLoaderFactory.newDataLoader(petBatchLoader);
296+
DataLoader ownerDL = myState.ownerDL;
297+
DataLoader petDL = myState.petDL;
292298

293299
for (Owner owner : owners.values()) {
294300
ownerDL.load(owner.id);

‎src/main/java/org/dataloader/CacheMap.java‎

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -74,10 +74,11 @@ static <K, V> CacheMap<K, V> simpleMap() {
7474
*
7575
* @return the cached value, or {@code null} if not found (depends on cache implementation)
7676
*/
77-
@Nullable CompletableFuture<V> get(K key);
77+
@Nullable CompletableFuture<V> get(K key);
7878

7979
/**
8080
* Gets a collection of CompletableFutures from the cache map.
81+
*
8182
* @return the collection of cached values
8283
*/
8384
Collection<CompletableFuture<V>> getAll();
@@ -90,7 +91,7 @@ static <K, V> CacheMap<K, V> simpleMap() {
9091
*
9192
* @return the cache map for fluent coding
9293
*/
93-
CacheMap<K, V> set(K key, CompletableFuture<V> value);
94+
CompletableFuture<V> putIfAbsentAtomically(K key, CompletableFuture<V> value);
9495

9596
/**
9697
* Deletes the entry with the specified key from the cache map, if it exists.
@@ -107,4 +108,13 @@ static <K, V> CacheMap<K, V> simpleMap() {
107108
* @return the cache map for fluent coding
108109
*/
109110
CacheMap<K, V> clear();
111+
112+
/**
113+
* Returns the current size of the cache. This is not used by DataLoader directly
114+
* and intended for testing and debugging.
115+
* If a cache doesn't support it, it can throw an Exception.
116+
*
117+
* @return
118+
*/
119+
int size();
110120
}

0 commit comments

Comments
 (0)