From 3d28e16a844c9ec8578e527391548f01d4a31b9a Mon Sep 17 00:00:00 2001 From: Lachlan Roberts Date: Thu, 22 Aug 2024 08:18:49 +1000 Subject: [PATCH] changes from review Signed-off-by: Lachlan Roberts --- .../core/util/BindingMethodHolder.java | 69 --------- .../websocket/core/util/MethodHolder.java | 142 ++++++++++++++++-- .../core/util/NonBindingMethodHolder.java | 103 ------------- .../websocket/jmh/MethodHolderBenchmark.java | 4 +- 4 files changed, 135 insertions(+), 183 deletions(-) delete mode 100644 jetty-core/jetty-websocket/jetty-websocket-core-common/src/main/java/org/eclipse/jetty/websocket/core/util/BindingMethodHolder.java delete mode 100644 jetty-core/jetty-websocket/jetty-websocket-core-common/src/main/java/org/eclipse/jetty/websocket/core/util/NonBindingMethodHolder.java diff --git a/jetty-core/jetty-websocket/jetty-websocket-core-common/src/main/java/org/eclipse/jetty/websocket/core/util/BindingMethodHolder.java b/jetty-core/jetty-websocket/jetty-websocket-core-common/src/main/java/org/eclipse/jetty/websocket/core/util/BindingMethodHolder.java deleted file mode 100644 index b58f623a215..00000000000 --- a/jetty-core/jetty-websocket/jetty-websocket-core-common/src/main/java/org/eclipse/jetty/websocket/core/util/BindingMethodHolder.java +++ /dev/null @@ -1,69 +0,0 @@ -// -// ======================================================================== -// Copyright (c) 1995 Mort Bay Consulting Pty Ltd and others. -// -// This program and the accompanying materials are made available under the -// terms of the Eclipse Public License v. 2.0 which is available at -// https://www.eclipse.org/legal/epl-2.0, or the Apache License, Version 2.0 -// which is available at https://www.apache.org/licenses/LICENSE-2.0. -// -// SPDX-License-Identifier: EPL-2.0 OR Apache-2.0 -// ======================================================================== -// - -package org.eclipse.jetty.websocket.core.util; - -import java.lang.invoke.MethodHandle; -import java.lang.invoke.MethodHandles; - -class BindingMethodHolder implements MethodHolder -{ - public MethodHandle _methodHandle; - - public BindingMethodHolder(MethodHandle methodHandle) - { - _methodHandle = methodHandle; - } - - @Override - public Object invoke(Object... args) throws Throwable - { - return MethodHolder.doInvoke(_methodHandle, args); - } - - public MethodHandle getMethodHandler() - { - return _methodHandle; - } - - public Object invoke(Object o1, Object o2) throws Throwable - { - return MethodHolder.doInvoke(_methodHandle, o1, o2); - } - - @Override - public BindingMethodHolder bindTo(Object arg) - { - _methodHandle = _methodHandle.bindTo(arg); - return this; - } - - @Override - public MethodHolder bindTo(Object arg, int idx) - { - _methodHandle = MethodHandles.insertArguments(_methodHandle, idx, arg); - return this; - } - - @Override - public Class parameterType(int idx) - { - return _methodHandle.type().parameterType(idx); - } - - @Override - public Class returnType() - { - return _methodHandle.type().returnType(); - } -} diff --git a/jetty-core/jetty-websocket/jetty-websocket-core-common/src/main/java/org/eclipse/jetty/websocket/core/util/MethodHolder.java b/jetty-core/jetty-websocket/jetty-websocket-core-common/src/main/java/org/eclipse/jetty/websocket/core/util/MethodHolder.java index be053d5751e..6d81a90f209 100644 --- a/jetty-core/jetty-websocket/jetty-websocket-core-common/src/main/java/org/eclipse/jetty/websocket/core/util/MethodHolder.java +++ b/jetty-core/jetty-websocket/jetty-websocket-core-common/src/main/java/org/eclipse/jetty/websocket/core/util/MethodHolder.java @@ -14,6 +14,11 @@ package org.eclipse.jetty.websocket.core.util; import java.lang.invoke.MethodHandle; +import java.lang.invoke.MethodHandles; +import java.lang.invoke.WrongMethodTypeException; +import java.util.ArrayList; +import java.util.List; +import java.util.Objects; /** * An interface for managing invocations of methods whose arguments may need to be augmented, by @@ -33,19 +38,18 @@ import java.lang.invoke.MethodHandle; public interface MethodHolder { String METHOD_HOLDER_BINDING_PROPERTY = "jetty.websocket.methodholder.binding"; + boolean IS_BINDING = System.getProperty(METHOD_HOLDER_BINDING_PROPERTY) == null || Boolean.getBoolean(METHOD_HOLDER_BINDING_PROPERTY); static MethodHolder from(MethodHandle methodHandle) { - String property = System.getProperty(METHOD_HOLDER_BINDING_PROPERTY); - boolean binding = property == null || Boolean.parseBoolean(property); - return from(methodHandle, binding); + return from(methodHandle, IS_BINDING); } static MethodHolder from(MethodHandle methodHandle, boolean binding) { if (methodHandle == null) return null; - return binding ? new BindingMethodHolder(methodHandle) : new NonBindingMethodHolder(methodHandle); + return binding ? new Binding(methodHandle) : new NonBinding(methodHandle); } Object invoke(Object... args) throws Throwable; @@ -70,11 +74,6 @@ public interface MethodHolder throw new UnsupportedOperationException(); } - static Object doInvoke(MethodHandle methodHandle, Object arg1, Object arg2) throws Throwable - { - return methodHandle.invoke(arg1, arg2); - } - static Object doInvoke(MethodHandle methodHandle, Object... args) throws Throwable { switch (args.length) @@ -103,4 +102,129 @@ public interface MethodHolder return methodHandle.invokeWithArguments(args); } } + + class Binding implements MethodHolder + { + public MethodHandle _methodHandle; + + private Binding(MethodHandle methodHandle) + { + _methodHandle = methodHandle; + } + + @Override + public Object invoke(Object... args) throws Throwable + { + return doInvoke(_methodHandle, args); + } + + @Override + public Binding bindTo(Object arg) + { + _methodHandle = _methodHandle.bindTo(arg); + return this; + } + + @Override + public MethodHolder bindTo(Object arg, int idx) + { + _methodHandle = MethodHandles.insertArguments(_methodHandle, idx, arg); + return this; + } + + @Override + public Class parameterType(int idx) + { + return _methodHandle.type().parameterType(idx); + } + + @Override + public Class returnType() + { + return _methodHandle.type().returnType(); + } + } + + /** + * This implementation of {@link MethodHolder} is not thread safe. + * Mutual exclusion should be used when calling {@link #invoke(Object...)}, or this should only + * be invoked from a single thread. + */ + class NonBinding implements MethodHolder + { + private final MethodHandle _methodHandle; + private final Object[] _parameters; + private final List _unboundParamIndexes = new ArrayList<>(); + + private NonBinding(MethodHandle methodHandle) + { + _methodHandle = Objects.requireNonNull(methodHandle); + int numParams = methodHandle.type().parameterCount(); + _parameters = new Object[numParams]; + for (int i = 0; i < numParams; i++) + { + _unboundParamIndexes.add(i); + } + } + + @Override + public Object invoke(Object... args) throws Throwable + { + try + { + insertArguments(args); + return doInvoke(_methodHandle, _parameters); + } + finally + { + clearArguments(); + } + } + + @Override + public MethodHolder bindTo(Object arg, int idx) + { + _parameters[_unboundParamIndexes.get(idx)] = arg; + _unboundParamIndexes.remove(idx); + return this; + } + + @Override + public MethodHolder bindTo(Object arg) + { + return bindTo(arg, 0); + } + + private void insertArguments(Object... args) + { + if (_unboundParamIndexes.size() != args.length) + throw new WrongMethodTypeException(String.format("Expected %s params but had %s", _unboundParamIndexes.size(), args.length)); + + int argsIndex = 0; + for (int index : _unboundParamIndexes) + { + _parameters[index] = args[argsIndex++]; + } + } + + private void clearArguments() + { + for (int i : _unboundParamIndexes) + { + _parameters[i] = null; + } + } + + @Override + public Class parameterType(int idx) + { + return _methodHandle.type().parameterType(_unboundParamIndexes.get(idx)); + } + + @Override + public Class returnType() + { + return _methodHandle.type().returnType(); + } + } } diff --git a/jetty-core/jetty-websocket/jetty-websocket-core-common/src/main/java/org/eclipse/jetty/websocket/core/util/NonBindingMethodHolder.java b/jetty-core/jetty-websocket/jetty-websocket-core-common/src/main/java/org/eclipse/jetty/websocket/core/util/NonBindingMethodHolder.java deleted file mode 100644 index b92fa4c6672..00000000000 --- a/jetty-core/jetty-websocket/jetty-websocket-core-common/src/main/java/org/eclipse/jetty/websocket/core/util/NonBindingMethodHolder.java +++ /dev/null @@ -1,103 +0,0 @@ -// -// ======================================================================== -// Copyright (c) 1995 Mort Bay Consulting Pty Ltd and others. -// -// This program and the accompanying materials are made available under the -// terms of the Eclipse Public License v. 2.0 which is available at -// https://www.eclipse.org/legal/epl-2.0, or the Apache License, Version 2.0 -// which is available at https://www.apache.org/licenses/LICENSE-2.0. -// -// SPDX-License-Identifier: EPL-2.0 OR Apache-2.0 -// ======================================================================== -// - -package org.eclipse.jetty.websocket.core.util; - -import java.lang.invoke.MethodHandle; -import java.lang.invoke.WrongMethodTypeException; -import java.util.ArrayList; -import java.util.List; -import java.util.Objects; - -/** - * This implementation of {@link MethodHolder} is not thread safe. - * Mutual exclusion should be used when calling {@link #invoke(Object...)}, or this should only - * be invoked from a single thread. - */ -class NonBindingMethodHolder implements MethodHolder -{ - private final MethodHandle _methodHandle; - private final Object[] _parameters; - private final List _unboundParamIndexes = new ArrayList<>(); - - public NonBindingMethodHolder(MethodHandle methodHandle) - { - _methodHandle = Objects.requireNonNull(methodHandle); - int numParams = methodHandle.type().parameterCount(); - _parameters = new Object[numParams]; - for (int i = 0; i < numParams; i++) - { - _unboundParamIndexes.add(i); - } - } - - @Override - public Object invoke(Object... args) throws Throwable - { - try - { - insertArguments(args); - return MethodHolder.doInvoke(_methodHandle, _parameters); - } - finally - { - clearArguments(); - } - } - - @Override - public MethodHolder bindTo(Object arg, int idx) - { - _parameters[_unboundParamIndexes.get(idx)] = arg; - _unboundParamIndexes.remove(idx); - return this; - } - - @Override - public MethodHolder bindTo(Object arg) - { - return bindTo(arg, 0); - } - - private void insertArguments(Object... args) - { - if (_unboundParamIndexes.size() != args.length) - throw new WrongMethodTypeException(String.format("Expected %s params but had %s", _unboundParamIndexes.size(), args.length)); - - int argsIndex = 0; - for (int index : _unboundParamIndexes) - { - _parameters[index] = args[argsIndex++]; - } - } - - private void clearArguments() - { - for (int i : _unboundParamIndexes) - { - _parameters[i] = null; - } - } - - @Override - public Class parameterType(int idx) - { - return _methodHandle.type().parameterType(_unboundParamIndexes.get(idx)); - } - - @Override - public Class returnType() - { - return _methodHandle.type().returnType(); - } -} diff --git a/tests/jetty-jmh/src/main/java/org/eclipse/jetty/websocket/jmh/MethodHolderBenchmark.java b/tests/jetty-jmh/src/main/java/org/eclipse/jetty/websocket/jmh/MethodHolderBenchmark.java index c531fd4ed48..f96ae788e27 100644 --- a/tests/jetty-jmh/src/main/java/org/eclipse/jetty/websocket/jmh/MethodHolderBenchmark.java +++ b/tests/jetty-jmh/src/main/java/org/eclipse/jetty/websocket/jmh/MethodHolderBenchmark.java @@ -98,8 +98,8 @@ public class MethodHolderBenchmark { Options opt = new OptionsBuilder() .include(MethodHolderBenchmark.class.getSimpleName()) - .warmupIterations(5) - .measurementIterations(10) + .warmupIterations(1) + .measurementIterations(5) .forks(1) .threads(1) .build();