Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -537,6 +537,7 @@
<include>**/MultiDb*.java</include>
<include>**/ClientTestUtil.java</include>
<include>**/ReflectionTestUtil.java</include>
<include>**/*CommandFlags*.java</include>
</includes>
</configuration>
<executions>
Expand Down
859 changes: 153 additions & 706 deletions src/main/java/redis/clients/jedis/StaticCommandFlagsRegistry.java

Large diffs are not rendered by default.

Large diffs are not rendered by default.

6 changes: 3 additions & 3 deletions src/main/java/redis/clients/jedis/UnifiedJedis.java
Original file line number Diff line number Diff line change
Expand Up @@ -172,21 +172,21 @@ public UnifiedJedis(Connection connection) {

@Deprecated
public UnifiedJedis(ClusterConnectionProvider provider, int maxAttempts, Duration maxTotalRetriesDuration) {
this(new ClusterCommandExecutor(provider, maxAttempts, maxTotalRetriesDuration, new StaticCommandFlagsRegistry()), provider,
this(new ClusterCommandExecutor(provider, maxAttempts, maxTotalRetriesDuration, StaticCommandFlagsRegistry.registry()), provider,
new ClusterCommandObjects());
}

@Deprecated
protected UnifiedJedis(ClusterConnectionProvider provider, int maxAttempts, Duration maxTotalRetriesDuration,
RedisProtocol protocol) {
this(new ClusterCommandExecutor(provider, maxAttempts, maxTotalRetriesDuration, new StaticCommandFlagsRegistry()), provider,
this(new ClusterCommandExecutor(provider, maxAttempts, maxTotalRetriesDuration, StaticCommandFlagsRegistry.registry()), provider,
new ClusterCommandObjects(), protocol);
}

@Deprecated
protected UnifiedJedis(ClusterConnectionProvider provider, int maxAttempts, Duration maxTotalRetriesDuration,
RedisProtocol protocol, Cache cache) {
this(new ClusterCommandExecutor(provider, maxAttempts, maxTotalRetriesDuration, new StaticCommandFlagsRegistry()), provider,
this(new ClusterCommandExecutor(provider, maxAttempts, maxTotalRetriesDuration, StaticCommandFlagsRegistry.registry()), provider,
new ClusterCommandObjects(), protocol, cache);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ protected ConnectionProvider createDefaultConnectionProvider() {
* @return CommandFlagsRegistry
*/
protected CommandFlagsRegistry createDefaultCommandFlagsRegistry() {
return new StaticCommandFlagsRegistry();
return StaticCommandFlagsRegistry.registry();
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ public class ClusterCommandExecutor implements CommandExecutor {
@Deprecated
public ClusterCommandExecutor(ClusterConnectionProvider provider, int maxAttempts,
Duration maxTotalRetriesDuration) {
this(provider, maxAttempts, maxTotalRetriesDuration, new StaticCommandFlagsRegistry());
this(provider, maxAttempts, maxTotalRetriesDuration, StaticCommandFlagsRegistry.registry());
}

public ClusterCommandExecutor(ClusterConnectionProvider provider, int maxAttempts,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ public class ClusterCommandExecutorTest {
public void runSuccessfulExecute() {
ClusterConnectionProvider connectionHandler = mock(ClusterConnectionProvider.class);
ClusterCommandExecutor testMe = new ClusterCommandExecutor(connectionHandler, 10, Duration.ZERO,
new StaticCommandFlagsRegistry()) {
StaticCommandFlagsRegistry.registry()) {
@Override
public <T> T execute(Connection connection, CommandObject<T> commandObject) {
return (T) "foo";
Expand All @@ -55,7 +55,7 @@ protected void sleep(long ignored) {
public void runFailOnFirstExecSuccessOnSecondExec() {
ClusterConnectionProvider connectionHandler = mock(ClusterConnectionProvider.class);
ClusterCommandExecutor testMe = new ClusterCommandExecutor(connectionHandler, 10, ONE_SECOND,
new StaticCommandFlagsRegistry()) {
StaticCommandFlagsRegistry.registry()) {
boolean isFirstCall = true;

@Override
Expand All @@ -82,7 +82,7 @@ public void runAlwaysFailing() {
ClusterConnectionProvider connectionHandler = mock(ClusterConnectionProvider.class);
final LongConsumer sleep = mock(LongConsumer.class);
ClusterCommandExecutor testMe = new ClusterCommandExecutor(connectionHandler, 3, ONE_SECOND,
new StaticCommandFlagsRegistry()) {
StaticCommandFlagsRegistry.registry()) {
@Override
public <T> T execute(Connection connection, CommandObject<T> commandObject) {
throw new JedisConnectionException("Connection failed");
Expand Down Expand Up @@ -113,7 +113,7 @@ public void runMovedSuccess() {
ClusterConnectionProvider connectionHandler = mock(ClusterConnectionProvider.class);
final HostAndPort movedTarget = new HostAndPort(null, 0);
ClusterCommandExecutor testMe = new ClusterCommandExecutor(connectionHandler, 10, ONE_SECOND,
new StaticCommandFlagsRegistry()) {
StaticCommandFlagsRegistry.registry()) {
boolean isFirstCall = true;

@Override
Expand Down Expand Up @@ -151,7 +151,7 @@ public void runAskSuccess() {
when(connectionHandler.getConnection(askTarget)).thenReturn(connection);

ClusterCommandExecutor testMe = new ClusterCommandExecutor(connectionHandler, 10, ONE_SECOND,
new StaticCommandFlagsRegistry()) {
StaticCommandFlagsRegistry.registry()) {
boolean isFirstCall = true;

@Override
Expand Down Expand Up @@ -205,7 +205,7 @@ public void runMovedThenAllNodesFailing() {
final LongConsumer sleep = mock(LongConsumer.class);
final HostAndPort movedTarget = new HostAndPort(null, 0);
ClusterCommandExecutor testMe = new ClusterCommandExecutor(connectionHandler, 5, ONE_SECOND,
new StaticCommandFlagsRegistry()) {
StaticCommandFlagsRegistry.registry()) {
@Override
public <T> T execute(Connection connection, CommandObject<T> commandObject) {
if (redirecter == connection) {
Expand Down Expand Up @@ -273,7 +273,7 @@ public void runMasterFailingReplicaRecovering() {

final AtomicLong totalSleepMs = new AtomicLong();
ClusterCommandExecutor testMe = new ClusterCommandExecutor(connectionHandler, 5, ONE_SECOND,
new StaticCommandFlagsRegistry()) {
StaticCommandFlagsRegistry.registry()) {

@Override
public <T> T execute(Connection connection, CommandObject<T> commandObject) {
Expand Down Expand Up @@ -311,7 +311,7 @@ public void runRethrowsJedisNoReachableClusterNodeException() {
JedisClusterOperationException.class);

ClusterCommandExecutor testMe = new ClusterCommandExecutor(connectionHandler, 10,
Duration.ZERO, new StaticCommandFlagsRegistry()) {
Duration.ZERO, StaticCommandFlagsRegistry.registry()) {
@Override
public <T> T execute(Connection connection, CommandObject<T> commandObject) {
return null;
Expand All @@ -333,7 +333,7 @@ public void runStopsRetryingAfterTimeout() {
//final LongConsumer sleep = mock(LongConsumer.class);
final AtomicLong totalSleepMs = new AtomicLong();
ClusterCommandExecutor testMe = new ClusterCommandExecutor(connectionHandler, 3, Duration.ZERO,
new StaticCommandFlagsRegistry()) {
StaticCommandFlagsRegistry.registry()) {
@Override
public <T> T execute(Connection connection, CommandObject<T> commandObject) {
try {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ public class StaticCommandFlagsRegistryTest {

@BeforeEach
public void setUp() {
registry = new StaticCommandFlagsRegistry();
registry = StaticCommandFlagsRegistry.registry();
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@
*/
public class CommandFlagsRegistryGenerator {

private static final String JAVA_FILE = "src/main/java/redis/clients/jedis/StaticCommandFlagsRegistry.java";
private static final String JAVA_FILE = "src/main/java/redis/clients/jedis/StaticCommandFlagsRegistryInitializer.java";
private static final String BACKUP_JSON_FILE = "redis_commands_flags.json";

private final String redisHost;
Expand Down Expand Up @@ -316,10 +316,8 @@ private String generateRegistryClass(Map<FlagSet, List<String>> flagCombinations
// Package and imports
sb.append("package redis.clients.jedis;\n\n");
sb.append("import java.util.EnumSet;\n");
sb.append("import redis.clients.jedis.args.Rawable;\n");
sb.append("import redis.clients.jedis.commands.ProtocolCommand;\n");
sb.append("import redis.clients.jedis.util.JedisByteMap;\n");
sb.append("import redis.clients.jedis.util.SafeEncoder;\n\n");
sb.append("import static redis.clients.jedis.StaticCommandFlagsRegistry.EMPTY_FLAGS;\n");
sb.append("import static redis.clients.jedis.CommandFlagsRegistry.CommandFlag;\n");

// Class javadoc
sb.append("/**\n");
Expand All @@ -344,42 +342,10 @@ private String generateRegistryClass(Map<FlagSet, List<String>> flagCombinations
}

sb.append(" */\n");
sb.append("public class StaticCommandFlagsRegistry implements CommandFlagsRegistry {\n\n");

// Empty flags constant
sb.append(" // Empty flags constant for commands with no flags\n");
sb.append(
" private static final EnumSet<CommandFlag> EMPTY_FLAGS = EnumSet.noneOf(CommandFlag.class);\n\n");

// Command flags registry map - now hierarchical and using byte arrays for faster lookup
sb.append(" // Hierarchical command flags registry using byte arrays as keys\n");
sb.append(
" // Maps parent command names (as uppercase byte arrays) to their flags and subcommands\n");
sb.append(
" // For simple commands (no subcommands), the parent command maps directly to flags\n");
sb.append(
" // For parent commands with subcommands, the parent maps to a SubcommandRegistry\n");
sb.append(
" // Using JedisByteMap avoids String conversion during lookup, providing ~2.5x performance improvement\n");
sb.append(
" private static final JedisByteMap<Object> COMMAND_FLAGS_REGISTRY = new JedisByteMap<>();\n\n");

// SubcommandRegistry inner class
sb.append(" /**\n");
sb.append(" * Internal class to hold subcommand mappings for parent commands.\n");
sb.append(" * Uses JedisByteMap for faster subcommand lookup without String conversion.\n");
sb.append(" */\n");
sb.append(" private static class SubcommandRegistry {\n");
sb.append(" final EnumSet<CommandFlag> parentFlags;\n");
sb.append(" final JedisByteMap<EnumSet<CommandFlag>> subcommands;\n\n");
sb.append(" SubcommandRegistry(EnumSet<CommandFlag> parentFlags) {\n");
sb.append(" this.parentFlags = parentFlags;\n");
sb.append(" this.subcommands = new JedisByteMap<>();\n");
sb.append(" }\n");
sb.append(" }\n\n");
sb.append("final class StaticCommandFlagsRegistryInitializer {\n\n");

// Static initializer block
sb.append(" static {\n");
sb.append(" static void initialize(StaticCommandFlagsRegistry.Builder builder) {\n");

// Organize commands into parent commands and simple commands
Map<String, Map<String, FlagSet>> parentCommands = new LinkedHashMap<>();
Expand Down Expand Up @@ -416,30 +382,21 @@ private String generateRegistryClass(Map<FlagSet, List<String>> flagCombinations

// Generate parent command registries
for (String parent : knownParents) {
sb.append(String.format("builder.register(\"%s\", EMPTY_FLAGS);", parent));

Map<String, FlagSet> subcommands = parentCommands.get(parent);
if (subcommands != null && !subcommands.isEmpty()) {
sb.append(String.format(" // %s parent command with subcommands\n", parent));

// Determine parent flags (use EMPTY_FLAGS for now, could be enhanced)
sb.append(String.format(
" SubcommandRegistry %sRegistry = new SubcommandRegistry(EMPTY_FLAGS);\n",
parent.toLowerCase()));

// Add subcommands
List<String> sortedSubcommands = new ArrayList<>(subcommands.keySet());
Collections.sort(sortedSubcommands);

for (String subcommand : sortedSubcommands) {
FlagSet flagSet = subcommands.get(subcommand);
String enumSetExpr = createEnumSetExpression(flagSet.flags);
sb.append(
String.format(" %sRegistry.subcommands.put(SafeEncoder.encode(\"%s\"), %s);\n",
parent.toLowerCase(), subcommand, enumSetExpr));
sb.append(String.format("builder.register(\"%s\", \"%s\", %s);\n", parent, subcommand,
enumSetExpr));
}

sb.append(String.format(
" COMMAND_FLAGS_REGISTRY.put(SafeEncoder.encode(\"%s\"), %sRegistry);\n\n", parent,
parent.toLowerCase()));
}
}

Expand Down Expand Up @@ -472,97 +429,14 @@ private String generateRegistryClass(Map<FlagSet, List<String>> flagCombinations

// Add registry entries using SafeEncoder.encode()
for (String command : commands) {
sb.append(String.format(" COMMAND_FLAGS_REGISTRY.put(SafeEncoder.encode(\"%s\"), %s);\n",
command, enumSetExpr));
sb.append(String.format("builder.register(\"%s\", %s);\n", command, enumSetExpr));
}
sb.append("\n");
}

// Close initializer block
sb.append(" }\n\n");

// getFlags method implementation
sb.append(" /**\n");
sb.append(" * Get the flags for a given command. Flags are looked up from a static\n");
sb.append(
" * registry based on the command arguments. This approach significantly reduces\n");
sb.append(" * memory usage by sharing flag instances across all CommandObject instances.\n");
sb.append(" * <p>\n");
sb.append(
" * For commands with subcommands (e.g., FUNCTION LOAD, ACL SETUSER), this method\n");
sb.append(" * implements a hierarchical lookup strategy:\n");
sb.append(" * <ol>\n");
sb.append(
" * <li>First, retrieve the parent command using CommandArguments.getCommand()</li>\n");
sb.append(
" * <li>Check if this is a parent command (has subcommands in the registry)</li>\n");
sb.append(" * <li>If it is a parent command, attempt to get the child/subcommand by:\n");
sb.append(" * <ul>\n");
sb.append(
" * <li>Extracting the second argument from the CommandArguments object</li>\n");
sb.append(
" * <li>Matching this second argument against the child items of the parent command</li>\n");
sb.append(" * </ul>\n");
sb.append(" * </li>\n");
sb.append(
" * <li>Return the appropriate flags based on whether a child command was found or just use the parent command's flags</li>\n");
sb.append(" * </ol>\n");
sb.append(" *\n");
sb.append(
" * @param commandArguments the command arguments containing the command and its parameters\n");
sb.append(
" * @return EnumSet of CommandFlag for this command, or empty set if command has no flags\n");
sb.append(" */\n");
sb.append(" @Override\n");
sb.append(" public EnumSet<CommandFlag> getFlags(CommandArguments commandArguments) {\n");
sb.append(" // Get the parent command\n");
sb.append(" ProtocolCommand cmd = commandArguments.getCommand();\n");
sb.append(" byte[] raw = cmd.getRaw();\n");
sb.append(" \n");
sb.append(
" // Convert to uppercase using SafeEncoder utility (faster than String.toUpperCase())\n");
sb.append(" byte[] uppercaseBytes = SafeEncoder.toUpperCase(raw);\n");
sb.append("\n");
sb.append(" // Look up the parent command in the registry using byte array key\n");
sb.append(" Object registryEntry = COMMAND_FLAGS_REGISTRY.get(uppercaseBytes);\n");
sb.append("\n");
sb.append(" if (registryEntry == null) {\n");
sb.append(" // Command not found in registry\n");
sb.append(" return EMPTY_FLAGS;\n");
sb.append(" }\n");
sb.append("\n");
sb.append(
" // Check if this is a simple command (EnumSet) or a parent command with subcommands (SubcommandRegistry)\n");
sb.append(" if (registryEntry instanceof EnumSet) {\n");
sb.append(" // Simple command without subcommands\n");
sb.append(" return (EnumSet<CommandFlag>) registryEntry;\n");
sb.append(" } else if (registryEntry instanceof SubcommandRegistry) {\n");
sb.append(" // Parent command with subcommands\n");
sb.append(
" SubcommandRegistry subcommandRegistry = (SubcommandRegistry) registryEntry;\n");
sb.append("\n");
sb.append(" // Try to extract the subcommand from the second argument\n");
sb.append(" if (commandArguments.size() > 1) {\n");
sb.append(" Rawable secondArg = commandArguments.get(1);\n");
sb.append(" byte[] subRaw = secondArg.getRaw();\n");
sb.append(" \n");
sb.append(" // Convert to uppercase using SafeEncoder utility\n");
sb.append(" byte[] subUppercaseBytes = SafeEncoder.toUpperCase(subRaw);\n");
sb.append(" \n");
sb.append(
" EnumSet<CommandFlag> subcommandFlags = subcommandRegistry.subcommands.get(subUppercaseBytes);\n");
sb.append(" if (subcommandFlags != null) {\n");
sb.append(" return subcommandFlags;\n");
sb.append(" }\n");
sb.append(" }\n");
sb.append("\n");
sb.append(" // No subcommand found or no second argument, return parent flags\n");
sb.append(" return subcommandRegistry.parentFlags;\n");
sb.append(" }\n");
sb.append("\n");
sb.append(" // Should not reach here, but return empty flags as fallback\n");
sb.append(" return EMPTY_FLAGS;\n");
sb.append(" }\n");

// Close class
sb.append("}\n");

Expand Down
Loading