From 7c79bfb38d9f4f459a97e5ab1110dd186d4eb867 Mon Sep 17 00:00:00 2001 From: intsuc Date: Sat, 12 Aug 2023 00:28:04 +0900 Subject: [PATCH 1/4] Optimize parsing This optimization avoids unnecessary object allocations by not taking snapshots of the parsing state when the relevant nodes are unique. --- .../mojang/brigadier/CommandDispatcher.java | 21 +++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/src/main/java/com/mojang/brigadier/CommandDispatcher.java b/src/main/java/com/mojang/brigadier/CommandDispatcher.java index ef1f1237..7423edd3 100644 --- a/src/main/java/com/mojang/brigadier/CommandDispatcher.java +++ b/src/main/java/com/mojang/brigadier/CommandDispatcher.java @@ -354,13 +354,15 @@ private ParseResults parseNodes(final CommandNode node, final StringReader Map, CommandSyntaxException> errors = null; List> potentials = null; final int cursor = originalReader.getCursor(); + final Collection> relevantNodes = node.getRelevantNodes(originalReader); + final boolean unique = relevantNodes.size() == 1; - for (final CommandNode child : node.getRelevantNodes(originalReader)) { + for (final CommandNode child : relevantNodes) { if (!child.canUse(source)) { continue; } - final CommandContextBuilder context = contextSoFar.copy(); - final StringReader reader = new StringReader(originalReader); + final CommandContextBuilder context = unique ? contextSoFar : contextSoFar.copy(); + final StringReader reader = unique ? originalReader : new StringReader(originalReader); try { try { child.parse(reader, context); @@ -391,16 +393,23 @@ private ParseResults parseNodes(final CommandNode node, final StringReader return new ParseResults<>(context, parse.getReader(), parse.getExceptions()); } else { final ParseResults parse = parseNodes(child, reader, context); + if (unique) { + return parse; + } if (potentials == null) { - potentials = new ArrayList<>(1); + potentials = new ArrayList<>(relevantNodes.size()); } potentials.add(parse); } } else { + final ParseResults 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); } } From 12ce0d8f6a4d5694d5d72425f97cd0de0aeee15c Mon Sep 17 00:00:00 2001 From: intsuc Date: Sat, 12 Aug 2023 00:33:47 +0900 Subject: [PATCH 2/4] Avoid potential dead-code elimination According to JMH's JMHSamples_08_DeadCode, the correct way of benchmarking is either returning the result or consuming it with Blackholes. --- .../benchmarks/ParsingBenchmarks.java | 27 ++++++++----------- 1 file changed, 11 insertions(+), 16 deletions(-) diff --git a/src/test/java/com/mojang/brigadier/benchmarks/ParsingBenchmarks.java b/src/test/java/com/mojang/brigadier/benchmarks/ParsingBenchmarks.java index 712b3480..ae44cd19 100644 --- a/src/test/java/com/mojang/brigadier/benchmarks/ParsingBenchmarks.java +++ b/src/test/java/com/mojang/brigadier/benchmarks/ParsingBenchmarks.java @@ -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; @@ -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 subject; @@ -92,30 +95,22 @@ public void setup() { } @Benchmark - @BenchmarkMode(Mode.AverageTime) - @OutputTimeUnit(TimeUnit.NANOSECONDS) - public void parse_a1i() { - subject.parse("a 1 i", new Object()); + public ParseResults parse_a1i() { + return subject.parse("a 1 i", new Object()); } @Benchmark - @BenchmarkMode(Mode.AverageTime) - @OutputTimeUnit(TimeUnit.NANOSECONDS) - public void parse_c() { - subject.parse("c", new Object()); + public ParseResults parse_c() { + return subject.parse("c", new Object()); } @Benchmark - @BenchmarkMode(Mode.AverageTime) - @OutputTimeUnit(TimeUnit.NANOSECONDS) - public void parse_k1i() { - subject.parse("k 1 i", new Object()); + public ParseResults parse_k1i() { + return subject.parse("k 1 i", new Object()); } @Benchmark - @BenchmarkMode(Mode.AverageTime) - @OutputTimeUnit(TimeUnit.NANOSECONDS) - public void parse_() { - subject.parse("c", new Object()); + public ParseResults parse_() { + return subject.parse("c", new Object()); } } From f4d27ab04177908254b9b5d058a3ac6f9810a105 Mon Sep 17 00:00:00 2001 From: intsuc Date: Sat, 12 Aug 2023 00:36:13 +0900 Subject: [PATCH 3/4] Add benchmark with a large node size --- .../benchmarks/ParsingBenchmarks.java | 27 +++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/src/test/java/com/mojang/brigadier/benchmarks/ParsingBenchmarks.java b/src/test/java/com/mojang/brigadier/benchmarks/ParsingBenchmarks.java index ae44cd19..07fcbc52 100644 --- a/src/test/java/com/mojang/brigadier/benchmarks/ParsingBenchmarks.java +++ b/src/test/java/com/mojang/brigadier/benchmarks/ParsingBenchmarks.java @@ -92,6 +92,28 @@ 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 @@ -113,4 +135,9 @@ public ParseResults parse_k1i() { public ParseResults parse_() { return subject.parse("c", new Object()); } + + @Benchmark + public ParseResults parse_l123456789() { + return subject.parse("l 1 2 3 4 5 6 7 8 9", new Object()); + } } From 5749ff5ab261ca7b354ed633b0e0bd0454c2cc07 Mon Sep 17 00:00:00 2001 From: intsuc Date: Fri, 25 Aug 2023 12:59:39 +0900 Subject: [PATCH 4/4] Merge conditions --- .../java/com/mojang/brigadier/CommandDispatcher.java | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/main/java/com/mojang/brigadier/CommandDispatcher.java b/src/main/java/com/mojang/brigadier/CommandDispatcher.java index 7423edd3..f358a153 100644 --- a/src/main/java/com/mojang/brigadier/CommandDispatcher.java +++ b/src/main/java/com/mojang/brigadier/CommandDispatcher.java @@ -361,8 +361,15 @@ private ParseResults parseNodes(final CommandNode node, final StringReader if (!child.canUse(source)) { continue; } - final CommandContextBuilder context = unique ? contextSoFar : contextSoFar.copy(); - final StringReader reader = unique ? originalReader : new StringReader(originalReader); + final CommandContextBuilder context; + final StringReader reader; + if (unique) { + context = contextSoFar; + reader = originalReader; + } else { + context = contextSoFar.copy(); + reader = new StringReader(originalReader); + } try { try { child.parse(reader, context);