From 9fbf7bfe4294136f5157f9e373c9b90c41d7f79e Mon Sep 17 00:00:00 2001 From: Nathan Adams Date: Wed, 17 Jan 2018 12:13:29 +0100 Subject: [PATCH] Distinguish forked redirect vs regular redirect --- build.gradle | 2 +- .../mojang/brigadier/CommandDispatcher.java | 4 +- .../brigadier/builder/ArgumentBuilder.java | 23 +++++++- .../builder/LiteralArgumentBuilder.java | 2 +- .../builder/RequiredArgumentBuilder.java | 2 +- .../brigadier/context/CommandContext.java | 10 +++- .../context/CommandContextBuilder.java | 5 +- .../brigadier/tree/ArgumentCommandNode.java | 6 +- .../mojang/brigadier/tree/CommandNode.java | 8 ++- .../brigadier/tree/LiteralCommandNode.java | 6 +- .../brigadier/tree/RootCommandNode.java | 2 +- .../brigadier/CommandDispatcherTest.java | 2 +- .../benchmarks/ExecuteBenchmarks.java | 57 +++++++++++++++++++ .../benchmarks/RedirectedCommand.java | 44 -------------- .../brigadier/benchmarks/SimpleCommand.java | 36 ------------ 15 files changed, 108 insertions(+), 101 deletions(-) create mode 100644 src/test/java/com/mojang/brigadier/benchmarks/ExecuteBenchmarks.java delete mode 100644 src/test/java/com/mojang/brigadier/benchmarks/RedirectedCommand.java delete mode 100644 src/test/java/com/mojang/brigadier/benchmarks/SimpleCommand.java diff --git a/build.gradle b/build.gradle index e1b4a33..521a630 100644 --- a/build.gradle +++ b/build.gradle @@ -3,7 +3,7 @@ import groovy.io.FileType apply plugin: 'java-library' apply plugin: 'maven' -version = '0.1.18' +version = '0.1.19' group = 'com.mojang' task wrapper(type: Wrapper) { diff --git a/src/main/java/com/mojang/brigadier/CommandDispatcher.java b/src/main/java/com/mojang/brigadier/CommandDispatcher.java index ff21301..f7f8bb5 100644 --- a/src/main/java/com/mojang/brigadier/CommandDispatcher.java +++ b/src/main/java/com/mojang/brigadier/CommandDispatcher.java @@ -101,6 +101,7 @@ public class CommandDispatcher { final CommandContext context = contexts.get(i); final CommandContext child = context.getChild(); if (child != null) { + forked |= context.isForked(); if (!child.getNodes().isEmpty()) { foundCommand = true; final RedirectModifier modifier = context.getRedirectModifier(); @@ -112,9 +113,6 @@ public class CommandDispatcher { } else { final Collection results = modifier.apply(context); if (!results.isEmpty()) { - if (results.size() > 1) { - forked = true; - } if (next == null) { next = new ArrayList<>(results.size()); } diff --git a/src/main/java/com/mojang/brigadier/builder/ArgumentBuilder.java b/src/main/java/com/mojang/brigadier/builder/ArgumentBuilder.java index a8ef68f..327b3e7 100644 --- a/src/main/java/com/mojang/brigadier/builder/ArgumentBuilder.java +++ b/src/main/java/com/mojang/brigadier/builder/ArgumentBuilder.java @@ -2,10 +2,13 @@ package com.mojang.brigadier.builder; import com.mojang.brigadier.Command; import com.mojang.brigadier.RedirectModifier; +import com.mojang.brigadier.context.CommandContext; import com.mojang.brigadier.tree.CommandNode; import com.mojang.brigadier.tree.RootCommandNode; import java.util.Collection; +import java.util.Collections; +import java.util.function.Function; import java.util.function.Predicate; public abstract class ArgumentBuilder> { @@ -14,6 +17,7 @@ public abstract class ArgumentBuilder> { private Predicate requirement = s -> true; private CommandNode target; private RedirectModifier modifier = null; + private boolean forks; protected abstract T getThis(); @@ -56,15 +60,24 @@ public abstract class ArgumentBuilder> { } public T redirect(final CommandNode target) { - return redirect(target, null); + return forward(target, null, false); } - public T redirect(final CommandNode target, final RedirectModifier modifier) { + public T redirect(final CommandNode target, final Function, S> modifier) { + return forward(target, modifier == null ? null : o -> Collections.singleton(modifier.apply(o)), false); + } + + public T fork(final CommandNode target, final RedirectModifier modifier) { + return forward(target, modifier, true); + } + + public T forward(final CommandNode target, final RedirectModifier modifier, final boolean fork) { if (!arguments.getChildren().isEmpty()) { - throw new IllegalStateException("Cannot redirect a node with children"); + throw new IllegalStateException("Cannot forward a node with children"); } this.target = target; this.modifier = modifier; + this.forks = fork; return getThis(); } @@ -76,5 +89,9 @@ public abstract class ArgumentBuilder> { return modifier; } + public boolean isFork() { + return forks; + } + public abstract CommandNode build(); } diff --git a/src/main/java/com/mojang/brigadier/builder/LiteralArgumentBuilder.java b/src/main/java/com/mojang/brigadier/builder/LiteralArgumentBuilder.java index ef47f8c..2958b1f 100644 --- a/src/main/java/com/mojang/brigadier/builder/LiteralArgumentBuilder.java +++ b/src/main/java/com/mojang/brigadier/builder/LiteralArgumentBuilder.java @@ -25,7 +25,7 @@ public class LiteralArgumentBuilder extends ArgumentBuilder build() { - final LiteralCommandNode result = new LiteralCommandNode<>(getLiteral(), getCommand(), getRequirement(), getRedirect(), getRedirectModifier()); + final LiteralCommandNode result = new LiteralCommandNode<>(getLiteral(), getCommand(), getRequirement(), getRedirect(), getRedirectModifier(), isFork()); for (final CommandNode argument : getArguments()) { result.addChild(argument); diff --git a/src/main/java/com/mojang/brigadier/builder/RequiredArgumentBuilder.java b/src/main/java/com/mojang/brigadier/builder/RequiredArgumentBuilder.java index 45125a1..25c4b1e 100644 --- a/src/main/java/com/mojang/brigadier/builder/RequiredArgumentBuilder.java +++ b/src/main/java/com/mojang/brigadier/builder/RequiredArgumentBuilder.java @@ -42,7 +42,7 @@ public class RequiredArgumentBuilder extends ArgumentBuilder build() { - final ArgumentCommandNode result = new ArgumentCommandNode<>(getName(), getType(), getCommand(), getRequirement(), getRedirect(), getRedirectModifier(), getSuggestionsProvider()); + final ArgumentCommandNode result = new ArgumentCommandNode<>(getName(), getType(), getCommand(), getRequirement(), getRedirect(), getRedirectModifier(), isFork(), getSuggestionsProvider()); for (final CommandNode argument : getArguments()) { result.addChild(argument); diff --git a/src/main/java/com/mojang/brigadier/context/CommandContext.java b/src/main/java/com/mojang/brigadier/context/CommandContext.java index e10906e..d439918 100644 --- a/src/main/java/com/mojang/brigadier/context/CommandContext.java +++ b/src/main/java/com/mojang/brigadier/context/CommandContext.java @@ -17,8 +17,9 @@ public class CommandContext { private final StringRange range; private final CommandContext child; private final RedirectModifier modifier; + private final boolean forks; - public CommandContext(final S source, final String input, final Map> arguments, final Command command, final Map, StringRange> nodes, final StringRange range, final CommandContext child, final RedirectModifier modifier) { + public CommandContext(final S source, final String input, final Map> arguments, final Command command, final Map, StringRange> nodes, final StringRange range, final CommandContext child, final RedirectModifier modifier, boolean forks) { this.source = source; this.input = input; this.arguments = arguments; @@ -27,13 +28,14 @@ public class CommandContext { this.range = range; this.child = child; this.modifier = modifier; + this.forks = forks; } public CommandContext copyFor(final S source) { if (this.source == source) { return this; } - return new CommandContext<>(source, input, arguments, command, nodes, range, child, modifier); + return new CommandContext<>(source, input, arguments, command, nodes, range, child, modifier, forks); } public CommandContext getChild() { @@ -113,4 +115,8 @@ public class CommandContext { public Map, StringRange> getNodes() { return nodes; } + + public boolean isForked() { + return forks; + } } diff --git a/src/main/java/com/mojang/brigadier/context/CommandContextBuilder.java b/src/main/java/com/mojang/brigadier/context/CommandContextBuilder.java index d84f4cd..6778591 100644 --- a/src/main/java/com/mojang/brigadier/context/CommandContextBuilder.java +++ b/src/main/java/com/mojang/brigadier/context/CommandContextBuilder.java @@ -17,6 +17,7 @@ public class CommandContextBuilder { private CommandContextBuilder child; private StringRange range; private RedirectModifier modifier = null; + private boolean forks; public CommandContextBuilder(final CommandDispatcher dispatcher, final S source, final int start) { this.dispatcher = dispatcher; @@ -51,6 +52,7 @@ public class CommandContextBuilder { nodes.put(node, range); this.range = StringRange.encompassing(this.range, range); this.modifier = node.getRedirectModifier(); + this.forks = node.isFork(); return this; } @@ -61,6 +63,7 @@ public class CommandContextBuilder { copy.nodes.putAll(nodes); copy.child = child; copy.range = range; + copy.forks = forks; return copy; } @@ -90,7 +93,7 @@ public class CommandContextBuilder { } public CommandContext build(final String input) { - return new CommandContext<>(source, input, arguments, command, nodes, range, child == null ? null : child.build(input), modifier); + return new CommandContext<>(source, input, arguments, command, nodes, range, child == null ? null : child.build(input), modifier, forks); } public CommandDispatcher getDispatcher() { diff --git a/src/main/java/com/mojang/brigadier/tree/ArgumentCommandNode.java b/src/main/java/com/mojang/brigadier/tree/ArgumentCommandNode.java index f89c8eb..e6d8820 100644 --- a/src/main/java/com/mojang/brigadier/tree/ArgumentCommandNode.java +++ b/src/main/java/com/mojang/brigadier/tree/ArgumentCommandNode.java @@ -24,8 +24,8 @@ public class ArgumentCommandNode extends CommandNode { private final ArgumentType type; private final SuggestionProvider customSuggestions; - public ArgumentCommandNode(final String name, final ArgumentType type, final Command command, final Predicate requirement, final CommandNode redirect, final RedirectModifier modifier, final SuggestionProvider customSuggestions) { - super(command, requirement, redirect, modifier); + public ArgumentCommandNode(final String name, final ArgumentType type, final Command command, final Predicate requirement, final CommandNode redirect, final RedirectModifier modifier, final boolean forks, final SuggestionProvider customSuggestions) { + super(command, requirement, redirect, modifier, forks); this.name = name; this.type = type; this.customSuggestions = customSuggestions; @@ -72,7 +72,7 @@ public class ArgumentCommandNode extends CommandNode { public RequiredArgumentBuilder createBuilder() { final RequiredArgumentBuilder builder = RequiredArgumentBuilder.argument(name, type); builder.requires(getRequirement()); - builder.redirect(getRedirect(), getRedirectModifier()); + builder.forward(getRedirect(), getRedirectModifier(), isFork()); builder.suggests(customSuggestions); if (getCommand() != null) { builder.executes(getCommand()); diff --git a/src/main/java/com/mojang/brigadier/tree/CommandNode.java b/src/main/java/com/mojang/brigadier/tree/CommandNode.java index 2d5e949..f90f172 100644 --- a/src/main/java/com/mojang/brigadier/tree/CommandNode.java +++ b/src/main/java/com/mojang/brigadier/tree/CommandNode.java @@ -27,13 +27,15 @@ public abstract class CommandNode implements Comparable> { private final Predicate requirement; private final CommandNode redirect; private final RedirectModifier modifier; + private final boolean forks; private Command command; - protected CommandNode(final Command command, final Predicate requirement, final CommandNode redirect, final RedirectModifier modifier) { + protected CommandNode(final Command command, final Predicate requirement, final CommandNode redirect, final RedirectModifier modifier, final boolean forks) { this.command = command; this.requirement = requirement; this.redirect = redirect; this.modifier = modifier; + this.forks = forks; } public Command getCommand() { @@ -144,4 +146,8 @@ public abstract class CommandNode implements Comparable> { .compare(getSortedKey(), o.getSortedKey()) .result(); } + + public boolean isFork() { + return forks; + } } diff --git a/src/main/java/com/mojang/brigadier/tree/LiteralCommandNode.java b/src/main/java/com/mojang/brigadier/tree/LiteralCommandNode.java index b94440f..3c20297 100644 --- a/src/main/java/com/mojang/brigadier/tree/LiteralCommandNode.java +++ b/src/main/java/com/mojang/brigadier/tree/LiteralCommandNode.java @@ -20,8 +20,8 @@ public class LiteralCommandNode extends CommandNode { private final String literal; - public LiteralCommandNode(final String literal, final Command command, final Predicate requirement, final CommandNode redirect, final RedirectModifier modifier) { - super(command, requirement, redirect, modifier); + public LiteralCommandNode(final String literal, final Command command, final Predicate requirement, final CommandNode redirect, final RedirectModifier modifier, final boolean forks) { + super(command, requirement, redirect, modifier, forks); this.literal = literal; } @@ -86,7 +86,7 @@ public class LiteralCommandNode extends CommandNode { public LiteralArgumentBuilder createBuilder() { final LiteralArgumentBuilder builder = LiteralArgumentBuilder.literal(this.literal); builder.requires(getRequirement()); - builder.redirect(getRedirect(), getRedirectModifier()); + builder.forward(getRedirect(), getRedirectModifier(), isFork()); if (getCommand() != null) { builder.executes(getCommand()); } diff --git a/src/main/java/com/mojang/brigadier/tree/RootCommandNode.java b/src/main/java/com/mojang/brigadier/tree/RootCommandNode.java index 848bf74..a0ac609 100644 --- a/src/main/java/com/mojang/brigadier/tree/RootCommandNode.java +++ b/src/main/java/com/mojang/brigadier/tree/RootCommandNode.java @@ -13,7 +13,7 @@ import java.util.concurrent.CompletableFuture; public class RootCommandNode extends CommandNode { public RootCommandNode() { - super(null, c -> true, null, s -> Collections.singleton(s.getSource())); + super(null, c -> true, null, s -> Collections.singleton(s.getSource()), false); } @Override diff --git a/src/test/java/com/mojang/brigadier/CommandDispatcherTest.java b/src/test/java/com/mojang/brigadier/CommandDispatcherTest.java index 1d4870e..ac0d741 100644 --- a/src/test/java/com/mojang/brigadier/CommandDispatcherTest.java +++ b/src/test/java/com/mojang/brigadier/CommandDispatcherTest.java @@ -281,7 +281,7 @@ public class CommandDispatcherTest { when(modifier.apply(argThat(hasProperty("source", is(source))))).thenReturn(Lists.newArrayList(source1, source2)); subject.register(literal("actual").executes(command)); - subject.register(literal("redirected").redirect(subject.getRoot(), modifier)); + subject.register(literal("redirected").redirect(subject.getRoot())); final String input = "redirected actual"; final ParseResults parse = subject.parse(input, source); diff --git a/src/test/java/com/mojang/brigadier/benchmarks/ExecuteBenchmarks.java b/src/test/java/com/mojang/brigadier/benchmarks/ExecuteBenchmarks.java new file mode 100644 index 0000000..57f8d04 --- /dev/null +++ b/src/test/java/com/mojang/brigadier/benchmarks/ExecuteBenchmarks.java @@ -0,0 +1,57 @@ +package com.mojang.brigadier.benchmarks; + +import com.google.common.collect.Lists; +import com.mojang.brigadier.CommandDispatcher; +import com.mojang.brigadier.ParseResults; +import com.mojang.brigadier.exceptions.CommandSyntaxException; +import org.openjdk.jmh.annotations.Benchmark; +import org.openjdk.jmh.annotations.BenchmarkMode; +import org.openjdk.jmh.annotations.Mode; +import org.openjdk.jmh.annotations.OutputTimeUnit; +import org.openjdk.jmh.annotations.Scope; +import org.openjdk.jmh.annotations.Setup; +import org.openjdk.jmh.annotations.State; + +import java.util.concurrent.TimeUnit; + +import static com.mojang.brigadier.builder.LiteralArgumentBuilder.literal; + +@State(Scope.Benchmark) +public class ExecuteBenchmarks { + private CommandDispatcher dispatcher; + private ParseResults simple; + private ParseResults singleRedirect; + private ParseResults forkedRedirect; + + @Setup + public void setup() { + dispatcher = new CommandDispatcher<>(); + dispatcher.register(literal("command").executes(c -> 0)); + dispatcher.register(literal("redirect").redirect(dispatcher.getRoot())); + dispatcher.register(literal("fork").fork(dispatcher.getRoot(), o -> Lists.newArrayList(new Object(), new Object(), new Object()))); + simple = dispatcher.parse("command", new Object()); + singleRedirect = dispatcher.parse("redirect command", new Object()); + forkedRedirect = dispatcher.parse("fork command", new Object()); + } + + @Benchmark + @BenchmarkMode(Mode.AverageTime) + @OutputTimeUnit(TimeUnit.NANOSECONDS) + public void execute_simple() throws CommandSyntaxException { + dispatcher.execute(simple); + } + + @Benchmark + @BenchmarkMode(Mode.AverageTime) + @OutputTimeUnit(TimeUnit.NANOSECONDS) + public void execute_single_redirect() throws CommandSyntaxException { + dispatcher.execute(singleRedirect); + } + + @Benchmark + @BenchmarkMode(Mode.AverageTime) + @OutputTimeUnit(TimeUnit.NANOSECONDS) + public void execute_forked_redirect() throws CommandSyntaxException { + dispatcher.execute(forkedRedirect); + } +} diff --git a/src/test/java/com/mojang/brigadier/benchmarks/RedirectedCommand.java b/src/test/java/com/mojang/brigadier/benchmarks/RedirectedCommand.java deleted file mode 100644 index 28826d6..0000000 --- a/src/test/java/com/mojang/brigadier/benchmarks/RedirectedCommand.java +++ /dev/null @@ -1,44 +0,0 @@ -package com.mojang.brigadier.benchmarks; - -import com.mojang.brigadier.CommandDispatcher; -import com.mojang.brigadier.ParseResults; -import com.mojang.brigadier.exceptions.CommandSyntaxException; -import com.mojang.brigadier.tree.LiteralCommandNode; -import org.openjdk.jmh.annotations.Benchmark; -import org.openjdk.jmh.annotations.BenchmarkMode; -import org.openjdk.jmh.annotations.Mode; -import org.openjdk.jmh.annotations.OutputTimeUnit; -import org.openjdk.jmh.annotations.Scope; -import org.openjdk.jmh.annotations.Setup; -import org.openjdk.jmh.annotations.State; - -import java.util.concurrent.TimeUnit; - -import static com.mojang.brigadier.builder.LiteralArgumentBuilder.literal; - -@State(Scope.Benchmark) -public class RedirectedCommand { - private CommandDispatcher dispatcher; - private ParseResults parse; - - public static void main(final String[] args) throws CommandSyntaxException { - final RedirectedCommand command = new RedirectedCommand(); - command.setup(); - command.execute(); - } - - @Setup - public void setup() { - dispatcher = new CommandDispatcher<>(); - dispatcher.register(literal("command").executes(c -> 0)); - dispatcher.register(literal("redirect").redirect(dispatcher.getRoot())); - parse = dispatcher.parse("redirect command", new Object()); - } - - @Benchmark - @BenchmarkMode(Mode.AverageTime) - @OutputTimeUnit(TimeUnit.NANOSECONDS) - public void execute() throws CommandSyntaxException { - dispatcher.execute(parse); - } -} diff --git a/src/test/java/com/mojang/brigadier/benchmarks/SimpleCommand.java b/src/test/java/com/mojang/brigadier/benchmarks/SimpleCommand.java deleted file mode 100644 index 229cbfc..0000000 --- a/src/test/java/com/mojang/brigadier/benchmarks/SimpleCommand.java +++ /dev/null @@ -1,36 +0,0 @@ -package com.mojang.brigadier.benchmarks; - -import com.mojang.brigadier.CommandDispatcher; -import com.mojang.brigadier.ParseResults; -import com.mojang.brigadier.exceptions.CommandSyntaxException; -import org.openjdk.jmh.annotations.Benchmark; -import org.openjdk.jmh.annotations.BenchmarkMode; -import org.openjdk.jmh.annotations.Mode; -import org.openjdk.jmh.annotations.OutputTimeUnit; -import org.openjdk.jmh.annotations.Scope; -import org.openjdk.jmh.annotations.Setup; -import org.openjdk.jmh.annotations.State; - -import java.util.concurrent.TimeUnit; - -import static com.mojang.brigadier.builder.LiteralArgumentBuilder.literal; - -@State(Scope.Benchmark) -public class SimpleCommand { - private CommandDispatcher dispatcher; - private ParseResults parse; - - @Setup - public void setup() { - dispatcher = new CommandDispatcher<>(); - dispatcher.register(literal("command").executes(c -> 0)); - parse = dispatcher.parse("command", new Object()); - } - - @Benchmark - @BenchmarkMode(Mode.AverageTime) - @OutputTimeUnit(TimeUnit.NANOSECONDS) - public void execute() throws CommandSyntaxException { - dispatcher.execute(parse); - } -}