DATAREDIS-698 - Polishing.

Eagerly initialize known commands. Preallocate Jedis response builder. Replace list concatenation with array copy. Reject null arguments in execute(). Remove inversion through collections with direct byte array creation. Reorder arguments in signatures, visibility modifiers, Javadoc.

Original pull request: #283.
This commit is contained in:
Mark Paluch
2017-10-06 13:52:45 +02:00
parent f6a17fb268
commit 8adea79e67
9 changed files with 136 additions and 88 deletions

View File

@@ -3047,7 +3047,7 @@ public class DefaultStringRedisConnection implements StringRedisConnection, Deco
*/
@Override
public Object execute(String command) {
return execute(command, (byte[][]) null);
return execute(command, EMPTY_2D_BYTE_ARRAY);
}
/*

View File

@@ -15,7 +15,6 @@
*/
package org.springframework.data.redis.connection;
import java.util.ArrayList;
import java.util.Collection;
import java.util.List;
import java.util.Properties;
@@ -142,16 +141,22 @@ public interface DefaultedRedisClusterConnection extends RedisClusterConnection,
*/
@Nullable
@Override
@SuppressWarnings("unchecked")
default <T> T execute(String command, byte[] key, Collection<byte[]> args) {
Assert.notNull(command, "Command must not be null!");
Assert.notNull(key, "Key must not be null!");
Assert.notNull(args, "Args must not be null!");
ArrayList<byte[]> allArgs = new ArrayList();
allArgs.add(key);
allArgs.addAll(args);
byte[][] commandArgs = new byte[args.size() + 1][];
return (T) execute(command, allArgs.toArray(new byte[allArgs.size()][]));
commandArgs[0] = key;
int targetIndex = 1;
for (byte[] binaryArgument : args) {
commandArgs[targetIndex++] = binaryArgument;
}
return (T) execute(command, commandArgs);
}
}

View File

@@ -623,17 +623,16 @@ public interface ReactiveHashCommands {
}
/**
* @return {@literal null} if not already set.
* @return the field.
*/
@Nullable
public ByteBuffer getField() {
return field;
}
}
/**
* Get the length of the value associated with {@code hashKey}. If either the {@code key} or the {@code hashKey} do
* not exist, {@code 0} is emitted.
* Get the length of the value associated with {@code field}. If either the {@code key} or the {@code field} do not
* exist, {@code 0} is emitted.
*
* @param key must not be {@literal null}.
* @param field must not be {@literal null}.
@@ -649,8 +648,8 @@ public interface ReactiveHashCommands {
}
/**
* Get the length of the value associated with {@code hashKey}. If either the {@code key} or the {@code hashKey} do
* not exist, {@code 0} is emitted.
* Get the length of the value associated with {@code field}. If either the {@code key} or the {@code field} do not
* exist, {@code 0} is emitted.
*
* @param commands must not be {@literal null}.
* @return never {@literal null}.

View File

@@ -65,7 +65,7 @@ public interface RedisClusterConnection extends RedisConnection, RedisClusterCom
* <pre>
* <code>
* // SET foo bar EX 10 NX
* execute("SET", "foo".getBytes(), asBinaryList("bar", "EX", 10, "NX")
* execute("SET", "foo".getBytes(), asBinaryList("bar", "EX", 10, "NX"))
* </code>
* </pre>
*

View File

@@ -29,26 +29,31 @@ import redis.clients.util.SafeEncoder;
import java.lang.reflect.Field;
import java.lang.reflect.Method;
import java.util.ArrayList;
import java.util.Collection;
import java.util.List;
import java.util.Arrays;
import java.util.Set;
import java.util.function.Supplier;
import java.util.stream.Collectors;
import org.springframework.beans.DirectFieldAccessor;
import org.springframework.lang.Nullable;
import org.springframework.util.ClassUtils;
import org.springframework.util.ReflectionUtils;
/**
* Utility class to dispatch arbitrary Redis commands using Jedis commands.
*
* @author Christoph Strobl
* @author Mark Paluch
* @since 2.1
*/
@SuppressWarnings({ "unchecked", "ConstantConditions" })
class JedisClientUtils {
private static final Field CLIENT_FIELD;
private static final Method SEND_COMMAND;
private static final Method GET_RESPONSE;
private static final Method PROTOCOL_SEND_COMMAND;
private static final Set<String> KNOWN_COMMANDS;
private static final Builder<Object> OBJECT_BUILDER;
static {
@@ -60,12 +65,12 @@ class JedisClientUtils {
ReflectionUtils.makeAccessible(PROTOCOL_SEND_COMMAND);
try {
Class<?> commandType = ClassUtils.isPresent("redis.clients.jedis.ProtocolCommand", null)
? ClassUtils.forName("redis.clients.jedis.ProtocolCommand", null)
: ClassUtils.forName("redis.clients.jedis.Protocol$Command", null);
SEND_COMMAND = ReflectionUtils.findMethod(Connection.class, "sendCommand",
new Class[] { commandType, byte[][].class });
SEND_COMMAND = ReflectionUtils.findMethod(Connection.class, "sendCommand", commandType, byte[][].class);
} catch (Exception e) {
throw new NoClassDefFoundError(
"Could not find required flavor of command required by 'redis.clients.jedis.Connection#sendCommand'.");
@@ -75,38 +80,56 @@ class JedisClientUtils {
GET_RESPONSE = ReflectionUtils.findMethod(Queable.class, "getResponse", Builder.class);
ReflectionUtils.makeAccessible(GET_RESPONSE);
KNOWN_COMMANDS = Arrays.stream(Command.values()).map(Enum::name).collect(Collectors.toSet());
OBJECT_BUILDER = new Builder<Object>() {
public Object build(Object data) {
return data;
}
public String toString() {
return "Object";
}
};
}
@Nullable
static <T> T execute(String command, Collection<byte[]> keys, Collection<byte[]> args, Supplier<Jedis> jedis) {
/**
* Execute an arbitrary on the supplied {@link Jedis} instance.
*
* @param command the command.
* @param keys must not be {@literal null}, may be empty.
* @param args must not be {@literal null}, may be empty.
* @param jedis must not be {@literal null}.
* @return the response, can be be {@literal null}.
*/
static <T> T execute(String command, byte[][] keys, byte[][] args, Supplier<Jedis> jedis) {
List<byte[]> mArgs = new ArrayList<>(keys);
mArgs.addAll(args);
byte[][] commandArgs = getCommandArguments(keys, args);
Client client = retrieveClient(jedis.get());
sendCommand(client, command, mArgs.toArray(new byte[mArgs.size()][]));
Client client = sendCommand(command, commandArgs, jedis.get());
return (T) client.getOne();
}
static Client retrieveClient(Jedis jedis) {
return (Client) ReflectionUtils.getField(CLIENT_FIELD, jedis);
}
static Client sendCommand(Jedis jedis, String command, byte[][] args) {
/**
* Send a Redis command and retrieve the {@link Client} for response retrieval.
*
* @param command the command.
* @param args must not be {@literal null}, may be empty.
* @param jedis must not be {@literal null}.
* @return the {@link Client} instance used to send the command.
*/
static Client sendCommand(String command, byte[][] args, Jedis jedis) {
Client client = retrieveClient(jedis);
if (isKnownCommand(command)) {
ReflectionUtils.invokeMethod(SEND_COMMAND, client, Command.valueOf(command.trim().toUpperCase()), args);
} else {
sendProtocolCommand(client, command, args);
}
sendCommand(client, command, args);
return client;
}
static void sendCommand(Client client, String command, byte[][] args) {
private static void sendCommand(Client client, String command, byte[][] args) {
if (isKnownCommand(command)) {
ReflectionUtils.invokeMethod(SEND_COMMAND, client, Command.valueOf(command.trim().toUpperCase()), args);
@@ -115,8 +138,9 @@ class JedisClientUtils {
}
}
static void sendProtocolCommand(Client client, String command, byte[][] args) {
private static void sendProtocolCommand(Client client, String command, byte[][] args) {
// quite expensive to construct for each command invocation
DirectFieldAccessor dfa = new DirectFieldAccessor(client);
client.connect();
@@ -125,34 +149,52 @@ class JedisClientUtils {
ReflectionUtils.invokeMethod(PROTOCOL_SEND_COMMAND, null, os, SafeEncoder.encode(command), args);
Integer pipelinedCommands = (Integer) dfa.getPropertyValue("pipelinedCommands");
dfa.setPropertyValue("pipelinedCommands", pipelinedCommands.intValue() + 1);
dfa.setPropertyValue("pipelinedCommands", pipelinedCommands + 1);
}
static boolean isKnownCommand(String command) {
private static boolean isKnownCommand(String command) {
return KNOWN_COMMANDS.contains(command);
}
try {
Command.valueOf(command);
return true;
} catch (IllegalArgumentException e) {
return false;
private static byte[][] getCommandArguments(byte[][] keys, byte[][] args) {
if (keys.length == 0) {
return args;
}
if (args.length == 0) {
return keys;
}
byte[][] commandArgs = new byte[keys.length + args.length][];
System.arraycopy(keys, 0, commandArgs, 0, keys.length);
System.arraycopy(args, 0, commandArgs, keys.length, args.length);
return commandArgs;
}
/**
* @param jedis the client instance.
* @return {@literal true} if the connection has entered {@literal MULTI} state.
*/
static boolean isInMulti(Jedis jedis) {
return retrieveClient(jedis).isInMulti();
}
static Response<Object> getGetResponse(Object target) {
return (Response<Object>) ReflectionUtils.invokeMethod(GET_RESPONSE, target, new Builder<Object>() {
public Object build(Object data) {
return data;
}
public String toString() {
return "Object";
}
});
/**
* Retrieve the {@link Response} object from a {@link redis.clients.jedis.Transaction} or a
* {@link redis.clients.jedis.Pipeline} for response synchronization.
*
* @param target a {@link redis.clients.jedis.Transaction} or {@link redis.clients.jedis.Pipeline}, must not be
* {@literal null}.
* @return the {@link Response} wrapper object.
*/
static Response<Object> getResponse(Object target) {
return (Response<Object>) ReflectionUtils.invokeMethod(GET_RESPONSE, target, OBJECT_BUILDER);
}
private static Client retrieveClient(Jedis jedis) {
return (Client) ReflectionUtils.getField(CLIENT_FIELD, jedis);
}
}

View File

@@ -23,10 +23,7 @@ import redis.clients.jedis.JedisCluster;
import redis.clients.jedis.JedisClusterConnectionHandler;
import redis.clients.jedis.JedisPool;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
import java.util.LinkedHashMap;
import java.util.LinkedHashSet;
import java.util.List;
@@ -70,6 +67,8 @@ public class JedisClusterConnection implements DefaultedRedisClusterConnection {
private static final ExceptionTranslationStrategy EXCEPTION_TRANSLATION = new FallbackExceptionTranslationStrategy(
JedisConverters.exceptionConverter());
private static final byte[][] EMPTY_2D_BYTE_ARRAY = new byte[0][];
private final Log log = LogFactory.getLog(getClass());
private final JedisCluster cluster;
@@ -130,14 +129,16 @@ public class JedisClusterConnection implements DefaultedRedisClusterConnection {
* (non-Javadoc)
* @see org.springframework.data.redis.connection.RedisCommands#execute(java.lang.String, byte[][])
*/
@Nullable
@Override
public Object execute(String command, byte[]... args) {
Assert.notNull(command, "Command must not be null!");
Assert.notNull(args, "Args must not be null!");
return clusterCommandExecutor
.executeCommandOnArbitraryNode((JedisClusterCommandCallback<Object>) client -> JedisClientUtils.execute(command,
Collections.emptyList(), Arrays.asList(args), () -> client))
EMPTY_2D_BYTE_ARRAY, args, () -> client))
.getValue();
}
@@ -153,14 +154,26 @@ public class JedisClusterConnection implements DefaultedRedisClusterConnection {
Assert.notNull(key, "Key must not be null!");
Assert.notNull(args, "Args must not be null!");
Collection<byte[]> commandArgs = new ArrayList<>();
commandArgs.add(key);
commandArgs.addAll(args);
byte[][] commandArgs = getCommandArguments(key, args);
RedisClusterNode keyMaster = topologyProvider.getTopology().getKeyServingMasterNode(key);
return clusterCommandExecutor.executeCommandOnSingleNode((JedisClusterCommandCallback<T>) client -> JedisClientUtils
.execute(command, Collections.emptyList(), commandArgs, () -> client), keyMaster).getValue();
.execute(command, EMPTY_2D_BYTE_ARRAY, commandArgs, () -> client), keyMaster).getValue();
}
private static byte[][] getCommandArguments(byte[] key, Collection<byte[]> args) {
byte[][] commandArgs = new byte[args.size() + 1][];
commandArgs[0] = key;
int targetIndex = 1;
for (byte[] binaryArgument : args) {
commandArgs[targetIndex++] = binaryArgument;
}
return commandArgs;
}
/*
@@ -816,7 +829,8 @@ public class JedisClusterConnection implements DefaultedRedisClusterConnection {
PropertyAccessor accessor = new DirectFieldAccessFallbackBeanWrapper(cluster);
this.connectionHandler = accessor.isReadableProperty("connectionHandler")
? (JedisClusterConnectionHandler) accessor.getPropertyValue("connectionHandler") : null;
? (JedisClusterConnectionHandler) accessor.getPropertyValue("connectionHandler")
: null;
} else {
this.connectionHandler = null;
}

View File

@@ -42,7 +42,6 @@ import org.springframework.data.redis.connection.convert.TransactionResultConver
import org.springframework.lang.Nullable;
import org.springframework.util.Assert;
import org.springframework.util.CollectionUtils;
import org.springframework.util.ObjectUtils;
import org.springframework.util.StringUtils;
/**
@@ -273,20 +272,18 @@ public class JedisConnection extends AbstractRedisConnection {
*/
@Override
public Object execute(String command, byte[]... args) {
Assert.hasText(command, "a valid command needs to be specified");
try {
List<byte[]> mArgs = new ArrayList<>();
if (!ObjectUtils.isEmpty(args)) {
Collections.addAll(mArgs, args);
}
Client client = JedisClientUtils.sendCommand(this.jedis, command, mArgs.toArray(new byte[mArgs.size()][]));
Assert.hasText(command, "A valid command needs to be specified!");
Assert.notNull(args, "Arguments must not be null!");
try {
Client client = JedisClientUtils.sendCommand(command, args, this.jedis);
if (isQueueing() || isPipelined()) {
Object target = (isPipelined() ? pipeline : transaction);
@SuppressWarnings("unchecked")
Response<Object> result = JedisClientUtils.getGetResponse(target);
Response<Object> result = JedisClientUtils
.getResponse(isPipelined() ? getRequiredPipeline() : getRequiredTransaction());
if (isPipelined()) {
pipeline(new JedisResult(result));
} else {

View File

@@ -224,8 +224,8 @@ class LettuceReactiveHashCommands implements ReactiveHashCommands {
return connection.execute(cmd -> Flux.from(commands).flatMap(command -> {
Assert.notNull(command.getKey(), "Command.getKey() must not be null!");
Assert.notNull(command.getField(), "Command.getField() must not be null!");
Assert.notNull(command.getKey(), "Key must not be null!");
Assert.notNull(command.getField(), "Field must not be null!");
return cmd.hstrlen(command.getKey(), command.getField()).map(value -> new NumericResponse<>(command, value));
}));