Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Initial POC commit for adding ReadOnlyMemory overloads to StringSet o… #2221

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

Initial POC commit for adding ReadOnlyMemory overloads to StringSet o… #2221

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
14 changes: 14 additions & 0 deletions src/StackExchange.Redis/Interfaces/IDatabase.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2864,6 +2864,20 @@ public interface IDatabase : IRedis, IDatabaseAsync
/// </remarks>
bool StringSet(KeyValuePair<RedisKey, RedisValue>[] values, When when = When.Always, CommandFlags flags = CommandFlags.None);

/// <summary>
/// Sets the given keys to their respective values.
/// If <see cref="When.NotExists"/> is specified, this will not perform any operation at all even if just a single key already exists.
/// </summary>
/// <param name="values">The keys and values to set.</param>
/// <param name="when">Which condition to set the value under (defaults to always).</param>
/// <param name="flags">The flags to use for this operation.</param>
/// <returns><see langword="true"/> if the keys were set, <see langword="false"/> otherwise.</returns>
/// <remarks>
/// <seealso href="https://redis.io/commands/mset"/>,
/// <seealso href="https://redis.io/commands/msetnx"/>
/// </remarks>
bool StringSet(ReadOnlyMemory<KeyValuePair<RedisKey, RedisValue>> values, When when = When.Always, CommandFlags flags = CommandFlags.None);

/// <summary>
/// Atomically sets key to value and returns the previous value (if any) stored at <paramref name="key"/>.
/// </summary>
Expand Down
14 changes: 14 additions & 0 deletions src/StackExchange.Redis/Interfaces/IDatabaseAsync.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2817,6 +2817,20 @@ public interface IDatabaseAsync : IRedisAsync
/// </remarks>
Task<bool> StringSetAsync(KeyValuePair<RedisKey, RedisValue>[] values, When when = When.Always, CommandFlags flags = CommandFlags.None);

/// <summary>
/// Sets the given keys to their respective values.
/// If <see cref="When.NotExists"/> is specified, this will not perform any operation at all even if just a single key already exists.
/// </summary>
/// <param name="values">The keys and values to set.</param>
/// <param name="when">Which condition to set the value under (defaults to always).</param>
/// <param name="flags">The flags to use for this operation.</param>
/// <returns><see langword="true"/> if the keys were set, <see langword="false"/> otherwise.</returns>
/// <remarks>
/// <seealso href="https://redis.io/commands/mset"/>,
/// <seealso href="https://redis.io/commands/msetnx"/>
/// </remarks>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

possibly worth a warning about FireAndForget being dangerous here if the ReadOnlyMemory is going to be recycled/reused?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Definitely seems wise, thank you. I'll add warnings for all of the new methods taking ReadOnlyMemory. If the approach looks good overall, I'll continue expanding the surface area.

Task<bool> StringSetAsync(ReadOnlyMemory<KeyValuePair<RedisKey, RedisValue>> values, When when = When.Always, CommandFlags flags = CommandFlags.None);

/// <summary>
/// Atomically sets key to value and returns the previous value (if any) stored at <paramref name="key"/>.
/// </summary>
Expand Down
3 changes: 3 additions & 0 deletions src/StackExchange.Redis/KeyspaceIsolation/DatabaseWrapper.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -670,6 +670,9 @@ public RedisResult Execute(string command, ICollection<object> args, CommandFlag
public bool StringSet(KeyValuePair<RedisKey, RedisValue>[] values, When when = When.Always, CommandFlags flags = CommandFlags.None) =>
Inner.StringSet(ToInner(values), when, flags);

public bool StringSet(ReadOnlyMemory<KeyValuePair<RedisKey, RedisValue>> values, When when = When.Always, CommandFlags flags = CommandFlags.None) =>
Inner.StringSet(ToInner(values), when, flags);

public bool StringSet(RedisKey key, RedisValue value, TimeSpan? expiry, When when) =>
Inner.StringSet(ToInner(key), value, expiry, when);
public bool StringSet(RedisKey key, RedisValue value, TimeSpan? expiry, When when, CommandFlags flags) =>
Expand Down
22 changes: 22 additions & 0 deletions src/StackExchange.Redis/KeyspaceIsolation/WrapperBase.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -686,6 +686,9 @@ internal WrapperBase(TInner inner, byte[] keyPrefix)
public Task<bool> StringSetAsync(KeyValuePair<RedisKey, RedisValue>[] values, When when = When.Always, CommandFlags flags = CommandFlags.None) =>
Inner.StringSetAsync(ToInner(values), when, flags);

public Task<bool> StringSetAsync(ReadOnlyMemory<KeyValuePair<RedisKey, RedisValue>> values, When when = When.Always, CommandFlags flags = CommandFlags.None) =>
Inner.StringSetAsync(ToInner(values), when, flags);

public Task<bool> StringSetAsync(RedisKey key, RedisValue value, TimeSpan? expiry, When when) =>
Inner.StringSetAsync(ToInner(key), value, expiry, when);
public Task<bool> StringSetAsync(RedisKey key, RedisValue value, TimeSpan? expiry, When when, CommandFlags flags) =>
Expand Down Expand Up @@ -804,6 +807,25 @@ internal WrapperBase(TInner inner, byte[] keyPrefix)
}
}

protected ReadOnlyMemory<KeyValuePair<RedisKey, RedisValue>> ToInner(ReadOnlyMemory<KeyValuePair<RedisKey, RedisValue>> outer)
{
if (outer.Length == 0)
{
return outer;
}
else
{
KeyValuePair<RedisKey, RedisValue>[] inner = new KeyValuePair<RedisKey, RedisValue>[outer.Length];

for (int i = 0; i < outer.Length; ++i)
{
inner[i] = ToInner(outer.Span[i]);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Accessing .Span is relatively expensive; this should be done outside the loop and resused.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The ++i is very unusual for C#, note

Copy link
Author

@Stabzs Stabzs Aug 26, 2022 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense, thanks! I'll make the changes to cache all call sites for .Span.

The ++i was copied from line 801 of the diff. I assumed that was intentional and copied it here. Is that an incorrect usage?

}

return inner;
}
}

protected RedisValue ToInner(RedisValue outer) =>
RedisKey.ConcatenateBytes(Prefix, null, (byte[]?)outer);

Expand Down
2 changes: 2 additions & 0 deletions src/StackExchange.Redis/PublicAPI.Shipped.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -715,6 +715,7 @@ StackExchange.Redis.IDatabase.StringSet(StackExchange.Redis.RedisKey key, StackE
StackExchange.Redis.IDatabase.StringSet(StackExchange.Redis.RedisKey key, StackExchange.Redis.RedisValue value, System.TimeSpan? expiry, StackExchange.Redis.When when) -> bool
StackExchange.Redis.IDatabase.StringSet(StackExchange.Redis.RedisKey key, StackExchange.Redis.RedisValue value, System.TimeSpan? expiry, StackExchange.Redis.When when, StackExchange.Redis.CommandFlags flags) -> bool
StackExchange.Redis.IDatabase.StringSet(System.Collections.Generic.KeyValuePair<StackExchange.Redis.RedisKey, StackExchange.Redis.RedisValue>[]! values, StackExchange.Redis.When when = StackExchange.Redis.When.Always, StackExchange.Redis.CommandFlags flags = StackExchange.Redis.CommandFlags.None) -> bool
StackExchange.Redis.IDatabase.StringSet(System.ReadOnlyMemory<System.Collections.Generic.KeyValuePair<StackExchange.Redis.RedisKey, StackExchange.Redis.RedisValue>> values, StackExchange.Redis.When when = StackExchange.Redis.When.Always, StackExchange.Redis.CommandFlags flags = StackExchange.Redis.CommandFlags.None) -> bool
StackExchange.Redis.IDatabase.StringSetAndGet(StackExchange.Redis.RedisKey key, StackExchange.Redis.RedisValue value, System.TimeSpan? expiry = null, bool keepTtl = false, StackExchange.Redis.When when = StackExchange.Redis.When.Always, StackExchange.Redis.CommandFlags flags = StackExchange.Redis.CommandFlags.None) -> StackExchange.Redis.RedisValue
StackExchange.Redis.IDatabase.StringSetAndGet(StackExchange.Redis.RedisKey key, StackExchange.Redis.RedisValue value, System.TimeSpan? expiry, StackExchange.Redis.When when, StackExchange.Redis.CommandFlags flags) -> StackExchange.Redis.RedisValue
StackExchange.Redis.IDatabase.StringSetBit(StackExchange.Redis.RedisKey key, long offset, bool bit, StackExchange.Redis.CommandFlags flags = StackExchange.Redis.CommandFlags.None) -> bool
Expand Down Expand Up @@ -944,6 +945,7 @@ StackExchange.Redis.IDatabaseAsync.StringSetAsync(StackExchange.Redis.RedisKey k
StackExchange.Redis.IDatabaseAsync.StringSetAsync(StackExchange.Redis.RedisKey key, StackExchange.Redis.RedisValue value, System.TimeSpan? expiry, StackExchange.Redis.When when) -> System.Threading.Tasks.Task<bool>!
StackExchange.Redis.IDatabaseAsync.StringSetAsync(StackExchange.Redis.RedisKey key, StackExchange.Redis.RedisValue value, System.TimeSpan? expiry, StackExchange.Redis.When when, StackExchange.Redis.CommandFlags flags) -> System.Threading.Tasks.Task<bool>!
StackExchange.Redis.IDatabaseAsync.StringSetAsync(System.Collections.Generic.KeyValuePair<StackExchange.Redis.RedisKey, StackExchange.Redis.RedisValue>[]! values, StackExchange.Redis.When when = StackExchange.Redis.When.Always, StackExchange.Redis.CommandFlags flags = StackExchange.Redis.CommandFlags.None) -> System.Threading.Tasks.Task<bool>!
StackExchange.Redis.IDatabaseAsync.StringSetAsync(System.ReadOnlyMemory<System.Collections.Generic.KeyValuePair<StackExchange.Redis.RedisKey, StackExchange.Redis.RedisValue>> values, StackExchange.Redis.When when = StackExchange.Redis.When.Always, StackExchange.Redis.CommandFlags flags = StackExchange.Redis.CommandFlags.None) -> System.Threading.Tasks.Task<bool>!
StackExchange.Redis.IDatabaseAsync.StringSetBitAsync(StackExchange.Redis.RedisKey key, long offset, bool bit, StackExchange.Redis.CommandFlags flags = StackExchange.Redis.CommandFlags.None) -> System.Threading.Tasks.Task<bool>!
StackExchange.Redis.IDatabaseAsync.StringSetRangeAsync(StackExchange.Redis.RedisKey key, long offset, StackExchange.Redis.RedisValue value, StackExchange.Redis.CommandFlags flags = StackExchange.Redis.CommandFlags.None) -> System.Threading.Tasks.Task<StackExchange.Redis.RedisValue>!
StackExchange.Redis.InternalErrorEventArgs
Expand Down
24 changes: 18 additions & 6 deletions src/StackExchange.Redis/RedisDatabase.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3125,6 +3125,12 @@ public bool StringSet(KeyValuePair<RedisKey, RedisValue>[] values, When when = W
return ExecuteSync(msg, ResultProcessor.Boolean);
}

public bool StringSet(ReadOnlyMemory<KeyValuePair<RedisKey, RedisValue>> values, When when = When.Always, CommandFlags flags = CommandFlags.None)
{
var msg = GetStringSetMessage(values, when, flags);
return ExecuteSync(msg, ResultProcessor.Boolean);
}

// Backwards compatibility overloads:
public Task<bool> StringSetAsync(RedisKey key, RedisValue value, TimeSpan? expiry, When when) =>
StringSetAsync(key, value, expiry, false, when);
Expand All @@ -3143,6 +3149,12 @@ public Task<bool> StringSetAsync(KeyValuePair<RedisKey, RedisValue>[] values, Wh
return ExecuteAsync(msg, ResultProcessor.Boolean);
}

public Task<bool> StringSetAsync(ReadOnlyMemory<KeyValuePair<RedisKey, RedisValue>> values, When when = When.Always, CommandFlags flags = CommandFlags.None)
{
var msg = GetStringSetMessage(values, when, flags);
return ExecuteAsync(msg, ResultProcessor.Boolean);
}

public RedisValue StringSetAndGet(RedisKey key, RedisValue value, TimeSpan? expiry, When when, CommandFlags flags) =>
StringSetAndGet(key, value, expiry, false, when, flags);

Expand Down Expand Up @@ -4356,23 +4368,23 @@ private Message GetStringGetWithExpiryMessage(RedisKey key, CommandFlags flags,
return new StringGetWithExpiryMessage(Database, flags, RedisCommand.TTL, key);
}

private Message? GetStringSetMessage(KeyValuePair<RedisKey, RedisValue>[] values, When when = When.Always, CommandFlags flags = CommandFlags.None)
private Message? GetStringSetMessage(ReadOnlyMemory<KeyValuePair<RedisKey, RedisValue>> values, When when = When.Always, CommandFlags flags = CommandFlags.None)
{
if (values == null) throw new ArgumentNullException(nameof(values));
if (values.Span == null) throw new ArgumentNullException(nameof(values));
switch (values.Length)
{
case 0: return null;
case 1: return GetStringSetMessage(values[0].Key, values[0].Value, null, false, when, flags);
case 1: return GetStringSetMessage(values.Span[0].Key, values.Span[0].Value, null, false, when, flags);
default:
WhenAlwaysOrNotExists(when);
int slot = ServerSelectionStrategy.NoSlot, offset = 0;
var args = new RedisValue[values.Length * 2];
var serverSelectionStrategy = multiplexer.ServerSelectionStrategy;
for (int i = 0; i < values.Length; i++)
{
args[offset++] = values[i].Key.AsRedisValue();
args[offset++] = values[i].Value;
slot = serverSelectionStrategy.CombineSlot(slot, values[i].Key);
args[offset++] = values.Span[i].Key.AsRedisValue();
args[offset++] = values.Span[i].Value;
slot = serverSelectionStrategy.CombineSlot(slot, values.Span[i].Key);
}
return Message.CreateInSlot(Database, slot, flags, when == When.NotExists ? RedisCommand.MSETNX : RedisCommand.MSET, args);
}
Expand Down