Commit 743d3ca8 authored by Marc Gravell's avatar Marc Gravell

make use of "in" for larger readonly structs on non-public APIs

parent 7de44835
......@@ -28,7 +28,7 @@ namespace StackExchange.Redis
/// <param name="obj">The <see cref="object"/> to compare.</param>
public override bool Equals(object obj) => obj is ChannelMessage cm
&& cm.Channel == Channel && cm.Message == Message;
internal ChannelMessage(ChannelMessageQueue queue, RedisChannel channel, RedisValue value)
internal ChannelMessage(ChannelMessageQueue queue, in RedisChannel channel, in RedisValue value)
{
_queue = queue;
Channel = channel;
......@@ -73,7 +73,7 @@ public sealed class ChannelMessageQueue
/// </summary>
public Task Completion => _queue.Reader.Completion;
internal ChannelMessageQueue(RedisChannel redisChannel, RedisSubscriber parent)
internal ChannelMessageQueue(in RedisChannel redisChannel, RedisSubscriber parent)
{
Channel = redisChannel;
_parent = parent;
......@@ -89,7 +89,9 @@ internal ChannelMessageQueue(RedisChannel redisChannel, RedisSubscriber parent)
internal void Subscribe(CommandFlags flags) => _parent.Subscribe(Channel, HandleMessage, flags);
internal Task SubscribeAsync(CommandFlags flags) => _parent.SubscribeAsync(Channel, HandleMessage, flags);
#pragma warning disable RCS1231 // Make parameter ref read-only. - uses as a delegate for Action<RedisChannel, RedisValue>
private void HandleMessage(RedisChannel channel, RedisValue value)
#pragma warning restore RCS1231 // Make parameter ref read-only.
{
var writer = _queue.Writer;
if (channel.IsNull && value.IsNull) // see ForSyncShutdown
......
......@@ -46,7 +46,9 @@ public override int GetHashCode()
}
public override bool Equals(object obj) => obj is CommandBytes cb && Equals(cb);
public bool Equals(CommandBytes other) => _0 == other._0 && _1 == other._1 && _2 == other._2;
bool IEquatable<CommandBytes>.Equals(CommandBytes other) => _0 == other._0 && _1 == other._1 && _2 == other._2;
public bool Equals(in CommandBytes other) => _0 == other._0 && _1 == other._1 && _2 == other._2;
// note: don't add == operators; with the implicit op above, that invalidates "==null" compiler checks (which should report a failure!)
......@@ -74,7 +76,9 @@ public unsafe int Length
public bool IsEmpty => _0 == 0L; // cheap way of checking zero length
#pragma warning disable RCS1231 // Make parameter ref read-only. - spans are tiny!
public unsafe void CopyTo(Span<byte> target)
#pragma warning restore RCS1231 // Make parameter ref read-only.
{
fixed (ulong* uPtr = &_0)
{
......@@ -114,7 +118,9 @@ public unsafe CommandBytes(string value)
}
}
#pragma warning disable RCS1231 // Make parameter ref read-only. - spans are tiny!
public unsafe CommandBytes(ReadOnlySpan<byte> value)
#pragma warning restore RCS1231 // Make parameter ref read-only.
{
if (value.Length > MaxLength) throw new ArgumentOutOfRangeException("Maximum command length exceeed: " + value.Length + " bytes");
_0 = _1 = _2 = 0L;
......@@ -125,7 +131,8 @@ public unsafe CommandBytes(ReadOnlySpan<byte> value)
*bPtr = (byte)UpperCasify(value.Length, bPtr + 1);
}
}
public unsafe CommandBytes(ReadOnlySequence<byte> value)
public unsafe CommandBytes(in ReadOnlySequence<byte> value)
{
if (value.Length > MaxLength) throw new ArgumentOutOfRangeException("Maximum command length exceeed");
int len = unchecked((int)value.Length);
......
......@@ -1896,7 +1896,7 @@ internal ServerEndPoint SelectServer(Message message)
return ServerSelectionStrategy.Select(message);
}
internal ServerEndPoint SelectServer(RedisCommand command, CommandFlags flags, RedisKey key)
internal ServerEndPoint SelectServer(RedisCommand command, CommandFlags flags, in RedisKey key)
{
return ServerSelectionStrategy.Select(command, key, flags);
}
......
......@@ -125,7 +125,9 @@ internal static Exception NoConnectionAvailable(bool includeDetail, bool include
return ex;
}
#pragma warning disable RCS1231 // Make parameter ref read-only. - spans are tiny!
internal static Exception PopulateInnerExceptions(ReadOnlySpan<ServerEndPoint> serverSnapshot)
#pragma warning restore RCS1231 // Make parameter ref read-only.
{
var innerExceptions = new List<Exception>();
......
......@@ -68,7 +68,7 @@ public enum GeoRadiusOptions
/// <param name="distance">Tthe distance from the result.</param>
/// <param name="hash">The hash of the result.</param>
/// <param name="position">The geo position of the result.</param>
internal GeoRadiusResult(RedisValue member, double? distance, long? hash, GeoPosition? position)
internal GeoRadiusResult(in RedisValue member, double? distance, long? hash, GeoPosition? position)
{
Member = member;
Distance = distance;
......@@ -140,21 +140,27 @@ public GeoPosition(double longitude, double latitude)
/// Compares two values for equality
/// </summary>
/// <param name="other">The <see cref="GeoPosition"/> to compare to.</param>
#pragma warning disable RCS1231 // Make parameter ref read-only. - public API
public bool Equals(GeoPosition other) => this == other;
#pragma warning restore RCS1231 // Make parameter ref read-only.
/// <summary>
/// Compares two values for equality
/// </summary>
/// <param name="x">The first position to compare.</param>
/// <param name="y">The second position to compare.</param>
#pragma warning disable RCS1231 // Make parameter ref read-only. - public API
public static bool operator ==(GeoPosition x, GeoPosition y) => x.Longitude == y.Longitude && x.Latitude == y.Latitude;
#pragma warning restore RCS1231 // Make parameter ref read-only.
/// <summary>
/// Compares two values for non-equality
/// </summary>
/// <param name="x">The first position to compare.</param>
/// <param name="y">The second position to compare.</param>
#pragma warning disable RCS1231 // Make parameter ref read-only. - public API
public static bool operator !=(GeoPosition x, GeoPosition y) => x.Longitude != y.Longitude || x.Latitude != y.Latitude;
#pragma warning restore RCS1231 // Make parameter ref read-only.
}
/// <summary>
......@@ -179,7 +185,9 @@ public GeoPosition(double longitude, double latitude)
/// <param name="longitude">The longitude position to use.</param>
/// <param name="latitude">The latitude position to use.</param>
/// <param name="member">The value to store for this position.</param>
#pragma warning disable RCS1231 // Make parameter ref read-only. - public API
public GeoEntry(double longitude, double latitude, RedisValue member)
#pragma warning restore RCS1231 // Make parameter ref read-only.
{
Member = member;
Position = new GeoPosition(longitude, latitude);
......@@ -222,13 +230,17 @@ public GeoEntry(double longitude, double latitude, RedisValue member)
/// </summary>
/// <param name="x">The first entry to compare.</param>
/// <param name="y">The second entry to compare.</param>
#pragma warning disable RCS1231 // Make parameter ref read-only. - public API
public static bool operator ==(GeoEntry x, GeoEntry y) => x.Position == y.Position && x.Member == y.Member;
#pragma warning restore RCS1231 // Make parameter ref read-only.
/// <summary>
/// Compares two values for non-equality
/// </summary>
/// <param name="x">The first entry to compare.</param>
/// <param name="y">The second entry to compare.</param>
#pragma warning disable RCS1231 // Make parameter ref read-only. - public API
public static bool operator !=(GeoEntry x, GeoEntry y) => x.Position != y.Position || x.Member != y.Member;
#pragma warning restore RCS1231 // Make parameter ref read-only.
}
}
This diff is collapsed.
......@@ -631,7 +631,7 @@ internal void SetUnknownDatabase()
currentDatabase = -1;
}
internal void Write(RedisKey key)
internal void Write(in RedisKey key)
{
var val = key.KeyValue;
if (val is string s)
......@@ -644,13 +644,13 @@ internal void Write(RedisKey key)
}
}
internal void Write(RedisChannel channel)
internal void Write(in RedisChannel channel)
=> WriteUnifiedPrefixedBlob(_ioPipe.Output, ChannelPrefix, channel.Value);
[MethodImpl(MethodImplOptions.AggressiveInlining)]
internal void WriteBulkString(RedisValue value)
internal void WriteBulkString(in RedisValue value)
=> WriteBulkString(value, _ioPipe.Output);
internal static void WriteBulkString(RedisValue value, PipeWriter output)
internal static void WriteBulkString(in RedisValue value, PipeWriter output)
{
switch (value.Type)
{
......@@ -889,7 +889,7 @@ private static void WriteUnifiedSpan(PipeWriter writer, ReadOnlySpan<byte> value
{
var span = writer.GetSpan(5 + MaxInt32TextLen + value.Length);
span[0] = (byte)'$';
int bytes = AppendToSpanSpan(span, value, 1);
int bytes = AppendToSpan(span, value, 1);
writer.Advance(bytes);
}
else
......@@ -906,7 +906,7 @@ private static void WriteUnifiedSpan(PipeWriter writer, ReadOnlySpan<byte> value
}
}
private static int AppendToSpanCommand(Span<byte> span, CommandBytes value, int offset = 0)
private static int AppendToSpanCommand(Span<byte> span, in CommandBytes value, int offset = 0)
{
span[offset++] = (byte)'$';
int len = value.Length;
......@@ -916,7 +916,9 @@ private static int AppendToSpanCommand(Span<byte> span, CommandBytes value, int
return WriteCrlf(span, offset);
}
private static int AppendToSpanSpan(Span<byte> span, ReadOnlySpan<byte> value, int offset = 0)
#pragma warning disable RCS1231 // Make parameter ref read-only. - spans are tiny
private static int AppendToSpan(Span<byte> span, ReadOnlySpan<byte> value, int offset = 0)
#pragma warning restore RCS1231 // Make parameter ref read-only.
{
offset = WriteRaw(span, value.Length, offset: offset);
value.CopyTo(span.Slice(offset, value.Length));
......@@ -1230,7 +1232,7 @@ internal async ValueTask<bool> ConnectedAsync(Socket socket, TextWriter log, Soc
}
}
private void MatchResult(RawResult result)
private void MatchResult(in RawResult result)
{
// check to see if it could be an out-of-band pubsub message
if (connectionType == ConnectionType.Subscription && result.Type == ResultType.MultiBulk)
......
......@@ -27,7 +27,7 @@ namespace StackExchange.Redis
private const ResultType NonNullFlag = (ResultType)128;
public RawResult(ResultType resultType, ReadOnlySequence<byte> payload, bool isNull)
public RawResult(ResultType resultType, in ReadOnlySequence<byte> payload, bool isNull)
{
switch (resultType)
{
......@@ -91,7 +91,7 @@ public override string ToString()
public Tokenizer GetEnumerator() => this;
private BufferReader _value;
public Tokenizer(ReadOnlySequence<byte> value)
public Tokenizer(in ReadOnlySequence<byte> value)
{
_value = new BufferReader(value);
Current = default;
......@@ -209,7 +209,7 @@ internal void Recycle(int limit = -1)
}
}
internal bool IsEqual(CommandBytes expected)
internal bool IsEqual(in CommandBytes expected)
{
if (expected.Length != Payload.Length) return false;
return new CommandBytes(Payload).Equals(expected);
......@@ -236,7 +236,7 @@ internal unsafe bool IsEqual(byte[] expected)
return true;
}
internal bool StartsWith(CommandBytes expected)
internal bool StartsWith(in CommandBytes expected)
{
var len = expected.Length;
if (len > Payload.Length) return false;
......
......@@ -56,73 +56,93 @@ private static bool DeterminePatternBased(byte[] value, PatternMode mode)
/// </summary>
/// <param name="x">The first <see cref="RedisChannel"/> to compare.</param>
/// <param name="y">The second <see cref="RedisChannel"/> to compare.</param>
#pragma warning disable RCS1231 // Make parameter ref read-only. - public API
public static bool operator !=(RedisChannel x, RedisChannel y) => !(x == y);
#pragma warning restore RCS1231 // Make parameter ref read-only.
/// <summary>
/// Indicate whether two channel names are not equal
/// </summary>
/// <param name="x">The first <see cref="RedisChannel"/> to compare.</param>
/// <param name="y">The second <see cref="RedisChannel"/> to compare.</param>
#pragma warning disable RCS1231 // Make parameter ref read-only. - public API
public static bool operator !=(string x, RedisChannel y) => !(x == y);
#pragma warning restore RCS1231 // Make parameter ref read-only.
/// <summary>
/// Indicate whether two channel names are not equal
/// </summary>
/// <param name="x">The first <see cref="RedisChannel"/> to compare.</param>
/// <param name="y">The second <see cref="RedisChannel"/> to compare.</param>
#pragma warning disable RCS1231 // Make parameter ref read-only. - public API
public static bool operator !=(byte[] x, RedisChannel y) => !(x == y);
#pragma warning restore RCS1231 // Make parameter ref read-only.
/// <summary>
/// Indicate whether two channel names are not equal
/// </summary>
/// <param name="x">The first <see cref="RedisChannel"/> to compare.</param>
/// <param name="y">The second <see cref="RedisChannel"/> to compare.</param>
#pragma warning disable RCS1231 // Make parameter ref read-only. - public API
public static bool operator !=(RedisChannel x, string y) => !(x == y);
#pragma warning restore RCS1231 // Make parameter ref read-only.
/// <summary>
/// Indicate whether two channel names are not equal
/// </summary>
/// <param name="x">The first <see cref="RedisChannel"/> to compare.</param>
/// <param name="y">The second <see cref="RedisChannel"/> to compare.</param>
#pragma warning disable RCS1231 // Make parameter ref read-only. - public API
public static bool operator !=(RedisChannel x, byte[] y) => !(x == y);
#pragma warning restore RCS1231 // Make parameter ref read-only.
/// <summary>
/// Indicate whether two channel names are equal
/// </summary>
/// <param name="x">The first <see cref="RedisChannel"/> to compare.</param>
/// <param name="y">The second <see cref="RedisChannel"/> to compare.</param>
#pragma warning disable RCS1231 // Make parameter ref read-only. - public API
public static bool operator ==(RedisChannel x, RedisChannel y) =>
x.IsPatternBased == y.IsPatternBased && RedisValue.Equals(x.Value, y.Value);
#pragma warning restore RCS1231 // Make parameter ref read-only.
/// <summary>
/// Indicate whether two channel names are equal
/// </summary>
/// <param name="x">The first <see cref="RedisChannel"/> to compare.</param>
/// <param name="y">The second <see cref="RedisChannel"/> to compare.</param>
#pragma warning disable RCS1231 // Make parameter ref read-only. - public API
public static bool operator ==(string x, RedisChannel y) =>
RedisValue.Equals(x == null ? null : Encoding.UTF8.GetBytes(x), y.Value);
#pragma warning restore RCS1231 // Make parameter ref read-only.
/// <summary>
/// Indicate whether two channel names are equal
/// </summary>
/// <param name="x">The first <see cref="RedisChannel"/> to compare.</param>
/// <param name="y">The second <see cref="RedisChannel"/> to compare.</param>
#pragma warning disable RCS1231 // Make parameter ref read-only. - public API
public static bool operator ==(byte[] x, RedisChannel y) => RedisValue.Equals(x, y.Value);
#pragma warning restore RCS1231 // Make parameter ref read-only.
/// <summary>
/// Indicate whether two channel names are equal
/// </summary>
/// <param name="x">The first <see cref="RedisChannel"/> to compare.</param>
/// <param name="y">The second <see cref="RedisChannel"/> to compare.</param>
#pragma warning disable RCS1231 // Make parameter ref read-only. - public API
public static bool operator ==(RedisChannel x, string y) =>
RedisValue.Equals(x.Value, y == null ? null : Encoding.UTF8.GetBytes(y));
#pragma warning restore RCS1231 // Make parameter ref read-only.
/// <summary>
/// Indicate whether two channel names are equal
/// </summary>
/// <param name="x">The first <see cref="RedisChannel"/> to compare.</param>
/// <param name="y">The second <see cref="RedisChannel"/> to compare.</param>
#pragma warning disable RCS1231 // Make parameter ref read-only. - public API
public static bool operator ==(RedisChannel x, byte[] y) => RedisValue.Equals(x.Value, y);
#pragma warning restore RCS1231 // Make parameter ref read-only.
/// <summary>
/// See Object.Equals
......@@ -223,13 +243,17 @@ public enum PatternMode
/// Obtain the channel name as a <see cref="T:byte[]"/>.
/// </summary>
/// <param name="key">The channel to get a byte[] from.</param>
#pragma warning disable RCS1231 // Make parameter ref read-only. - public API
public static implicit operator byte[] (RedisChannel key) => key.Value;
#pragma warning restore RCS1231 // Make parameter ref read-only.
/// <summary>
/// Obtain the channel name as a <see cref="string"/>.
/// </summary>
/// <param name="key">The channel to get a string from.</param>
#pragma warning disable RCS1231 // Make parameter ref read-only. - public API
public static implicit operator string (RedisChannel key)
#pragma warning restore RCS1231 // Make parameter ref read-only.
{
var arr = key.Value;
if (arr == null) return null;
......
......@@ -176,7 +176,7 @@ private static readonly RedisValue
COUNT = Encoding.ASCII.GetBytes("COUNT"),
ASC = Encoding.ASCII.GetBytes("ASC"),
DESC = Encoding.ASCII.GetBytes("DESC");
private Message GetGeoRadiusMessage(RedisKey key, RedisValue? member, double longitude, double latitude, double radius, GeoUnit unit, int count, Order? order, GeoRadiusOptions options, CommandFlags flags)
private Message GetGeoRadiusMessage(in RedisKey key, RedisValue? member, double longitude, double latitude, double radius, GeoUnit unit, int count, Order? order, GeoRadiusOptions options, CommandFlags flags)
{
var redisValues = new List<RedisValue>();
RedisCommand command;
......@@ -2419,7 +2419,7 @@ public Task<RedisValue> StringSetRangeAsync(RedisKey key, long offset, RedisValu
return ExecuteAsync(msg, ResultProcessor.RedisValue);
}
private Message GetExpiryMessage(RedisKey key, CommandFlags flags, TimeSpan? expiry, out ServerEndPoint server)
private Message GetExpiryMessage(in RedisKey key, CommandFlags flags, TimeSpan? expiry, out ServerEndPoint server)
{
TimeSpan duration;
if (expiry == null || (duration = expiry.Value) == TimeSpan.MaxValue)
......@@ -2441,7 +2441,7 @@ private Message GetExpiryMessage(RedisKey key, CommandFlags flags, TimeSpan? exp
return Message.Create(Database, flags, RedisCommand.EXPIRE, key, seconds);
}
private Message GetExpiryMessage(RedisKey key, CommandFlags flags, DateTime? expiry, out ServerEndPoint server)
private Message GetExpiryMessage(in RedisKey key, CommandFlags flags, DateTime? expiry, out ServerEndPoint server)
{
DateTime when;
if (expiry == null || (when = expiry.Value) == DateTime.MaxValue)
......@@ -3686,7 +3686,7 @@ private class StringGetWithExpiryMessage : Message.CommandKeyBase, IMultiMessage
private readonly RedisCommand ttlCommand;
private ResultBox<TimeSpan?> box;
public StringGetWithExpiryMessage(int db, CommandFlags flags, RedisCommand ttlCommand, RedisKey key)
public StringGetWithExpiryMessage(int db, CommandFlags flags, RedisCommand ttlCommand, in RedisKey key)
: base(db, flags, RedisCommand.GET, key)
{
this.ttlCommand = ttlCommand;
......
......@@ -29,7 +29,7 @@ public partial class ConnectionMultiplexer
return false;
}
internal Task AddSubscription(RedisChannel channel, Action<RedisChannel, RedisValue> handler, CommandFlags flags, object asyncState)
internal Task AddSubscription(in RedisChannel channel, Action<RedisChannel, RedisValue> handler, CommandFlags flags, object asyncState)
{
if (handler != null)
{
......@@ -52,7 +52,7 @@ internal Task AddSubscription(RedisChannel channel, Action<RedisChannel, RedisVa
return CompletedTask<bool>.Default(asyncState);
}
internal ServerEndPoint GetSubscribedServer(RedisChannel channel)
internal ServerEndPoint GetSubscribedServer(in RedisChannel channel)
{
if (!channel.IsNullOrEmpty)
{
......@@ -67,7 +67,7 @@ internal ServerEndPoint GetSubscribedServer(RedisChannel channel)
return null;
}
internal void OnMessage(RedisChannel subscription, RedisChannel channel, RedisValue payload)
internal void OnMessage(in RedisChannel subscription, in RedisChannel channel, in RedisValue payload)
{
ICompletable completable = null;
lock (subscriptions)
......@@ -100,7 +100,7 @@ internal Task RemoveAllSubscriptions(CommandFlags flags, object asyncState)
return last;
}
internal Task RemoveSubscription(RedisChannel channel, Action<RedisChannel, RedisValue> handler, CommandFlags flags, object asyncState)
internal Task RemoveSubscription(in RedisChannel channel, Action<RedisChannel, RedisValue> handler, CommandFlags flags, object asyncState)
{
lock (subscriptions)
{
......@@ -171,7 +171,7 @@ public ICompletable ForSyncShutdown()
var syncHandler = _syncHandler;
return syncHandler == null ? null : new MessageCompletable(default, default, syncHandler, null);
}
public ICompletable ForInvoke(RedisChannel channel, RedisValue message)
public ICompletable ForInvoke(in RedisChannel channel, in RedisValue message)
{
var syncHandler = _syncHandler;
var asyncHandler = _asyncHandler;
......@@ -193,7 +193,7 @@ public bool Remove(bool asAsync, Action<RedisChannel, RedisValue> value)
return _syncHandler == null && _asyncHandler == null;
}
public Task SubscribeToServer(ConnectionMultiplexer multiplexer, RedisChannel channel, CommandFlags flags, object asyncState, bool internalCall)
public Task SubscribeToServer(ConnectionMultiplexer multiplexer, in RedisChannel channel, CommandFlags flags, object asyncState, bool internalCall)
{
var cmd = channel.IsPatternBased ? RedisCommand.PSUBSCRIBE : RedisCommand.SUBSCRIBE;
var selected = multiplexer.SelectServer(cmd, flags, default(RedisKey));
......@@ -205,7 +205,7 @@ public Task SubscribeToServer(ConnectionMultiplexer multiplexer, RedisChannel ch
return selected.WriteDirectAsync(msg, ResultProcessor.TrackSubscriptions, asyncState);
}
public Task UnsubscribeFromServer(RedisChannel channel, CommandFlags flags, object asyncState, bool internalCall)
public Task UnsubscribeFromServer(in RedisChannel channel, CommandFlags flags, object asyncState, bool internalCall)
{
var oldOwner = Interlocked.Exchange(ref owner, null);
if (oldOwner == null) return null;
......@@ -218,7 +218,7 @@ public Task UnsubscribeFromServer(RedisChannel channel, CommandFlags flags, obje
internal ServerEndPoint GetOwner() => Volatile.Read(ref owner);
internal void Resubscribe(RedisChannel channel, ServerEndPoint server)
internal void Resubscribe(in RedisChannel channel, ServerEndPoint server)
{
if (server != null && Interlocked.CompareExchange(ref owner, server, server) == server)
{
......@@ -229,7 +229,7 @@ internal void Resubscribe(RedisChannel channel, ServerEndPoint server)
}
}
internal bool Validate(ConnectionMultiplexer multiplexer, RedisChannel channel)
internal bool Validate(ConnectionMultiplexer multiplexer, in RedisChannel channel)
{
bool changed = false;
var oldOwner = Volatile.Read(ref owner);
......
......@@ -122,7 +122,7 @@ public RespCommand(RedisCommandAttribute attrib, MethodInfo method, RespServer s
public bool HasSubCommands => _subcommands != null;
internal RespCommand WithSubCommands(RespCommand[] subs)
=> new RespCommand(this, subs);
private RespCommand(RespCommand parent, RespCommand[] subs)
private RespCommand(in RespCommand parent, RespCommand[] subs)
{
if (parent.IsSubCommand) throw new InvalidOperationException("Cannot have nested sub-commands");
if (parent.HasSubCommands) throw new InvalidOperationException("Already has sub-commands");
......@@ -155,7 +155,7 @@ public RespCommand Resolve(in RedisRequest request)
}
return this;
}
public TypedRedisValue Execute(RedisClient client, RedisRequest request)
public TypedRedisValue Execute(RedisClient client, in RedisRequest request)
{
var args = request.Count;
if (!CheckArity(request.Count))
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment