Skip to content

Commit

Permalink
Java: store pointers in scripts (#1795)
Browse files Browse the repository at this point in the history
Store script args and keys as pointers.

Signed-off-by: Yury-Fridlyand <[email protected]>
  • Loading branch information
Yury-Fridlyand authored Jul 4, 2024
1 parent 8ba7d37 commit f1167c1
Show file tree
Hide file tree
Showing 4 changed files with 64 additions and 12 deletions.
8 changes: 3 additions & 5 deletions java/client/src/main/java/glide/api/BaseClient.java
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,6 @@

import static glide.api.models.GlideString.gs;
import static glide.api.models.commands.SortBaseOptions.STORE_COMMAND_STRING;
import static glide.api.models.commands.bitmap.BitFieldOptions.BitFieldReadOnlySubCommands;
import static glide.api.models.commands.bitmap.BitFieldOptions.BitFieldSubCommands;
import static glide.api.models.commands.bitmap.BitFieldOptions.createBitFieldArgs;
import static glide.api.models.commands.bitmap.BitFieldOptions.createBitFieldGlideStringArgs;
import static glide.api.models.commands.stream.StreamClaimOptions.JUST_ID_REDIS_API;
Expand Down Expand Up @@ -1765,7 +1763,7 @@ public CompletableFuture<Long> pexpiretime(@NonNull GlideString key) {

@Override
public CompletableFuture<Object> invokeScript(@NonNull Script script) {
if (script.getBinarySafeOutput()) {
if (script.getBinaryOutput()) {
return commandManager.submitScript(
script, List.of(), List.of(), this::handleBinaryObjectOrNullResponse);
} else {
Expand All @@ -1777,7 +1775,7 @@ public CompletableFuture<Object> invokeScript(@NonNull Script script) {
@Override
public CompletableFuture<Object> invokeScript(
@NonNull Script script, @NonNull ScriptOptions options) {
if (script.getBinarySafeOutput()) {
if (script.getBinaryOutput()) {
return commandManager.submitScript(
script,
options.getKeys().stream().map(GlideString::gs).collect(Collectors.toList()),
Expand All @@ -1795,7 +1793,7 @@ public CompletableFuture<Object> invokeScript(
@Override
public CompletableFuture<Object> invokeScript(
@NonNull Script script, @NonNull ScriptOptionsGlideString options) {
if (script.getBinarySafeOutput()) {
if (script.getBinaryOutput()) {
return commandManager.submitScript(
script, options.getKeys(), options.getArgs(), this::handleBinaryObjectOrNullResponse);
} else {
Expand Down
10 changes: 5 additions & 5 deletions java/client/src/main/java/glide/api/models/Script.java
Original file line number Diff line number Diff line change
Expand Up @@ -20,18 +20,18 @@ public class Script implements AutoCloseable {

private boolean isDropped = false;

/** Indicatoin if script invocation output can return binary data. */
@Getter private final Boolean binarySafeOutput;
/** Indication if script invocation output can return binary data. */
@Getter private final Boolean binaryOutput;

/**
* Wraps around creating a Script object from <code>code</code>.
*
* @param code To execute with a ScriptInvoke call.
* @param binarySafeOutput Indicates if the output can return binary data.
* @param binaryOutput Indicates if the output can return binary data.
*/
public <T> Script(T code, Boolean binarySafeOutput) {
public <T> Script(T code, Boolean binaryOutput) {
this.hash = storeScript(GlideString.of(code).getBytes());
this.binarySafeOutput = binarySafeOutput;
this.binaryOutput = binaryOutput;
}

/** Drop the linked script from glide-rs <code>code</code>. */
Expand Down
22 changes: 20 additions & 2 deletions java/client/src/main/java/glide/managers/CommandManager.java
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
import redis_request.RedisRequestOuterClass.RequestType;
import redis_request.RedisRequestOuterClass.Routes;
import redis_request.RedisRequestOuterClass.ScriptInvocation;
import redis_request.RedisRequestOuterClass.ScriptInvocationPointers;
import redis_request.RedisRequestOuterClass.SimpleRoutes;
import redis_request.RedisRequestOuterClass.SlotTypes;
import response.ResponseOuterClass.Response;
Expand Down Expand Up @@ -161,7 +162,7 @@ public <T> CompletableFuture<T> submitScript(
List<GlideString> args,
RedisExceptionCheckedFunction<Response, T> responseHandler) {

RedisRequest.Builder command = prepareRedisRequest(script, keys, args);
RedisRequest.Builder command = prepareScript(script, keys, args);
return submitCommandToChannel(command, responseHandler);
}

Expand Down Expand Up @@ -285,8 +286,25 @@ protected RedisRequest.Builder prepareRedisRequest(Transaction transaction) {
* @return An uncompleted request. {@link CallbackDispatcher} is responsible to complete it by
* adding a callback id.
*/
protected RedisRequest.Builder prepareRedisRequest(
protected RedisRequest.Builder prepareScript(
Script script, List<GlideString> keys, List<GlideString> args) {

if (keys.stream().mapToLong(key -> key.getBytes().length).sum()
+ args.stream().mapToLong(key -> key.getBytes().length).sum()
> RedisValueResolver.MAX_REQUEST_ARGS_LENGTH_IN_BYTES) {
return RedisRequest.newBuilder()
.setScriptInvocationPointers(
ScriptInvocationPointers.newBuilder()
.setHash(script.getHash())
.setArgsPointer(
RedisValueResolver.createLeakedBytesVec(
args.stream().map(GlideString::getBytes).toArray(byte[][]::new)))
.setKeysPointer(
RedisValueResolver.createLeakedBytesVec(
keys.stream().map(GlideString::getBytes).toArray(byte[][]::new)))
.build());
}

return RedisRequest.newBuilder()
.setScriptInvocation(
ScriptInvocation.newBuilder()
Expand Down
36 changes: 36 additions & 0 deletions java/integTest/src/test/java/glide/SharedCommandTests.java
Original file line number Diff line number Diff line change
Expand Up @@ -3065,6 +3065,42 @@ public void invokeScript_test(BaseClient client) {
}
}

@SneakyThrows
@ParameterizedTest(autoCloseArguments = false)
@MethodSource("getClients")
public void script_large_keys_and_or_args(BaseClient client) {
String str1 = "0".repeat(1 << 12); // 4k
String str2 = "0".repeat(1 << 12); // 4k

try (Script script = new Script("return KEYS[1]", false)) {
// 1 very big key
Object response =
client.invokeScript(script, ScriptOptions.builder().key(str1 + str2).build()).get();
assertEquals(str1 + str2, response);
}

try (Script script = new Script("return KEYS[1]", false)) {
// 2 big keys
Object response =
client.invokeScript(script, ScriptOptions.builder().key(str1).key(str2).build()).get();
assertEquals(str1, response);
}

try (Script script = new Script("return ARGV[1]", false)) {
// 1 very big arg
Object response =
client.invokeScript(script, ScriptOptions.builder().arg(str1 + str2).build()).get();
assertEquals(str1 + str2, response);
}

try (Script script = new Script("return ARGV[1]", false)) {
// 1 big arg + 1 big key
Object response =
client.invokeScript(script, ScriptOptions.builder().arg(str1).key(str2).build()).get();
assertEquals(str2, response);
}
}

@SneakyThrows
@ParameterizedTest(autoCloseArguments = false)
@MethodSource("getClients")
Expand Down

0 comments on commit f1167c1

Please sign in to comment.