Skip to content

Commit afb6b72

Browse files
authored
[TINKERPOP-3202] Improve performance of RangeGlobalStep (#3251)
https://issues.apache.org/jira/browse/TINKERPOP-3202 Follow up to #3241 to improve performance of RangeGlobalStep by using single counter if not inside repeat and changing parent step check for RepeatStep to occur just once instead of for each call to filter. This change is for performance only and does not affect any semantics.
1 parent ddf21ee commit afb6b72

File tree

2 files changed

+94
-8
lines changed

2 files changed

+94
-8
lines changed

gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/filter/RangeGlobalStep.java

Lines changed: 40 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -46,9 +46,18 @@ public final class RangeGlobalStep<S> extends FilterStep<S> implements RangeGlob
4646
private long low;
4747
private long high;
4848
/**
49-
* If this range step is used inside a loop there can be multiple counters, otherwise there should only be one
49+
* Flag to indicate if the step is inside a repeat loop. Can be null if the value has not been initialized yet as
50+
* the traversal has not been finalized.
5051
*/
51-
private Map<String, AtomicLong> counters = new HashMap<>();
52+
private Boolean insideLoop;
53+
/**
54+
* Single counter if this range step is not inside a loop
55+
*/
56+
private AtomicLong singleCounter = new AtomicLong(0);
57+
/**
58+
* If this range step is used inside a loop there can be multiple loop counters
59+
*/
60+
private Map<String, AtomicLong> loopCounters = new HashMap<>();
5261
private boolean bypass;
5362

5463
public RangeGlobalStep(final Traversal.Admin traversal, final long low, final long high) {
@@ -64,11 +73,10 @@ public RangeGlobalStep(final Traversal.Admin traversal, final long low, final lo
6473
protected boolean filter(final Traverser.Admin<S> traverser) {
6574
if (this.bypass) return true;
6675

67-
final String counterKey = getCounterKey(traverser);
68-
final AtomicLong counter = counters.computeIfAbsent(counterKey, k -> new AtomicLong(0L));
76+
final AtomicLong counter = getCounter(traverser);
6977

7078
if (this.high != -1 && counter.get() >= this.high) {
71-
if (hasRepeatStepParent()) {
79+
if (isInsideLoop()) {
7280
return false;
7381
}
7482
throw FastNoSuchElementException.instance();
@@ -103,7 +111,8 @@ protected boolean filter(final Traverser.Admin<S> traverser) {
103111
@Override
104112
public void reset() {
105113
super.reset();
106-
this.counters.clear();
114+
this.singleCounter.set(0);
115+
this.loopCounters.clear();
107116
}
108117

109118
@Override
@@ -124,7 +133,8 @@ public Long getHighRange() {
124133
@Override
125134
public RangeGlobalStep<S> clone() {
126135
final RangeGlobalStep<S> clone = (RangeGlobalStep<S>) super.clone();
127-
clone.counters = new HashMap<>();
136+
clone.singleCounter = new AtomicLong(0);
137+
clone.loopCounters = new HashMap<>();
128138
return clone;
129139
}
130140

@@ -157,10 +167,32 @@ public void processAllStarts() {
157167

158168
}
159169

170+
private AtomicLong getCounter(final Traverser.Admin<S> traverser) {
171+
if (isInsideLoop()) {
172+
final String counterKey = getCounterKey(traverser);
173+
return loopCounters.computeIfAbsent(counterKey, k -> new AtomicLong(0L));
174+
} else {
175+
return this.singleCounter;
176+
}
177+
}
178+
179+
/**
180+
* This will initialize the insideLoop flag if it hasn't been set by analyzing the traversal up to the root and
181+
* should only be called after the traversal has been finalized.
182+
*
183+
* @return if the step is being used inside a repeat loop.
184+
*/
185+
private boolean isInsideLoop() {
186+
if (this.insideLoop == null) {
187+
this.insideLoop = hasRepeatStepParent();
188+
}
189+
return this.insideLoop;
190+
}
191+
160192
private String getCounterKey(final Traverser.Admin<S> traverser) {
161193
final List<String> counterKeyParts = new ArrayList<>();
162194
Traversal.Admin<Object, Object> traversal = this.getTraversal();
163-
if (hasRepeatStepParent()) {
195+
if (isInsideLoop()) {
164196
// the range step is inside a loop so we need to track counters per iteration
165197
// using a counter key that is composed of the parent steps to the root
166198
while (!traversal.isRoot()) {
Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one
3+
* or more contributor license agreements. See the NOTICE file
4+
* distributed with this work for additional information
5+
* regarding copyright ownership. The ASF licenses this file
6+
* to you under the Apache License, Version 2.0 (the
7+
* "License"); you may not use this file except in compliance
8+
* with the License. You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing,
13+
* software distributed under the License is distributed on an
14+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
* KIND, either express or implied. See the License for the
16+
* specific language governing permissions and limitations
17+
* under the License.
18+
*/
19+
package org.apache.tinkerpop.gremlin.process;
20+
21+
import java.util.List;
22+
import org.apache.tinkerpop.benchmark.util.AbstractGraphBenchmark;
23+
import org.apache.tinkerpop.gremlin.LoadGraphWith;
24+
import org.apache.tinkerpop.gremlin.structure.Vertex;
25+
import org.openjdk.jmh.annotations.Benchmark;
26+
27+
@LoadGraphWith(LoadGraphWith.GraphData.GRATEFUL)
28+
public class RangeTraversalBenchmark extends AbstractGraphBenchmark {
29+
30+
@Override
31+
protected int getWarmupIterations() {
32+
return 1;
33+
}
34+
35+
@Override
36+
protected int getForks() {
37+
return 1;
38+
}
39+
40+
@Benchmark
41+
public List<Vertex> limit() {
42+
return g.V().hasLabel("artist").limit(10).toList();
43+
}
44+
45+
@Benchmark
46+
public List<Vertex> range() {
47+
return g.V().hasLabel("artist").range(5, 20).toList();
48+
}
49+
50+
@Benchmark
51+
public List<Vertex> skip() {
52+
return g.V().hasLabel("artist").skip(5).toList();
53+
}
54+
}

0 commit comments

Comments
 (0)