Skip to content
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

Jump to bottom

Optimize parsing performance #135

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Optimize parsing performance #135

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
7c79bfb
Optimize parsing
intsuc Aug 11, 2023
12ce0d8
Avoid potential dead-code elimination
intsuc Aug 11, 2023
f4d27ab
Add benchmark with a large node size
intsuc Aug 11, 2023
5749ff5
Merge conditions
intsuc Aug 25, 2023
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
21 changes: 15 additions & 6 deletions src/main/java/com/mojang/brigadier/CommandDispatcher.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -354,13 +354,15 @@ private ParseResults<S> parseNodes(final CommandNode<S> node, final StringReader
Map<CommandNode<S>, CommandSyntaxException> errors = null;
List<ParseResults<S>> potentials = null;
final int cursor = originalReader.getCursor();
final Collection<? extends CommandNode<S>> relevantNodes = node.getRelevantNodes(originalReader);
final boolean unique = relevantNodes.size() == 1;

for (final CommandNode<S> child : node.getRelevantNodes(originalReader)) {
for (final CommandNode<S> child : relevantNodes) {
if (!child.canUse(source)) {
continue;
}
final CommandContextBuilder<S> context = contextSoFar.copy();
final StringReader reader = new StringReader(originalReader);
final CommandContextBuilder<S> context = unique ? contextSoFar : contextSoFar.copy();
final StringReader reader = unique ? originalReader : new StringReader(originalReader);
Copy link
Author

Choose a reason for hiding this comment

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

If unique, we do not need to take snapshots of the parsing state (contextSoFar and originalReader) here.

try {
try {
child.parse(reader, context);
Expand Down Expand Up @@ -391,16 +393,23 @@ private ParseResults<S> parseNodes(final CommandNode<S> node, final StringReader
return new ParseResults<>(context, parse.getReader(), parse.getExceptions());
} else {
final ParseResults<S> parse = parseNodes(child, reader, context);
if (unique) {
return parse;
}
Comment on lines +403 to +405
Copy link
Author

Choose a reason for hiding this comment

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

If unique, the potentials will always contain only parse and this parse will be returned after exiting this for loop, so we can return parse here without allocating potentials.

if (potentials == null) {
potentials = new ArrayList<>(1);
potentials = new ArrayList<>(relevantNodes.size());
Copy link
Author

Choose a reason for hiding this comment

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

Since potentials will contain ParseResults<S> up to the amount of relevantNodes.size(), allocate that capacity in advance to ensure resizing never occurs.

}
potentials.add(parse);
}
} else {
final ParseResults<S> parse = new ParseResults<>(context, reader, Collections.emptyMap());
if (unique) {
return parse;
}
if (potentials == null) {
potentials = new ArrayList<>(1);
potentials = new ArrayList<>(relevantNodes.size());
}
potentials.add(new ParseResults<>(context, reader, Collections.emptyMap()));
potentials.add(parse);
}
}

Expand Down
54 changes: 38 additions & 16 deletions src/test/java/com/mojang/brigadier/benchmarks/ParsingBenchmarks.java
View file
Open in desktop
Copy link

Choose a reason for hiding this comment

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

I think this is less readable. But if it increase performance it's ok.

Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
package com.mojang.brigadier.benchmarks;

import com.mojang.brigadier.CommandDispatcher;
import com.mojang.brigadier.ParseResults;
import com.mojang.brigadier.tree.LiteralCommandNode;
import org.openjdk.jmh.annotations.Benchmark;
import org.openjdk.jmh.annotations.BenchmarkMode;
Expand All @@ -18,6 +19,8 @@
import static com.mojang.brigadier.builder.LiteralArgumentBuilder.literal;

@State(Scope.Benchmark)
@BenchmarkMode(Mode.AverageTime)
@OutputTimeUnit(TimeUnit.NANOSECONDS)
public class ParsingBenchmarks {
private CommandDispatcher<Object> subject;

Expand Down Expand Up @@ -89,33 +92,52 @@ public void setup() {
literal("k")
.redirect(h)
);
subject.register(
literal("l")
.then(
literal("1")
.then(
literal("2")
.then(
literal("3")
.then(
literal("4")
.then(
literal("5")
.then(
literal("6")
.then(
literal("7")
.then(
literal("8")
.then(
literal("9")
.executes(c -> 0))))))))))
);
}

@Benchmark
public ParseResults<Object> parse_a1i() {
return subject.parse("a 1 i", new Object());
}

@Benchmark
@BenchmarkMode(Mode.AverageTime)
@OutputTimeUnit(TimeUnit.NANOSECONDS)
public void parse_a1i() {
subject.parse("a 1 i", new Object());
public ParseResults<Object> parse_c() {
return subject.parse("c", new Object());
}

@Benchmark
@BenchmarkMode(Mode.AverageTime)
@OutputTimeUnit(TimeUnit.NANOSECONDS)
public void parse_c() {
subject.parse("c", new Object());
public ParseResults<Object> parse_k1i() {
return subject.parse("k 1 i", new Object());
}

@Benchmark
@BenchmarkMode(Mode.AverageTime)
@OutputTimeUnit(TimeUnit.NANOSECONDS)
public void parse_k1i() {
subject.parse("k 1 i", new Object());
public ParseResults<Object> parse_() {
return subject.parse("c", new Object());
}

@Benchmark
@BenchmarkMode(Mode.AverageTime)
@OutputTimeUnit(TimeUnit.NANOSECONDS)
public void parse_() {
subject.parse("c", new Object());
public ParseResults<Object> parse_l123456789() {
return subject.parse("l 1 2 3 4 5 6 7 8 9", new Object());
}
}