Skip to content

Commit 8bbb138

Browse files
committed
Recycle ToStringHelper objects
This was the #2 allocated object in high traffic perf testing. Yet we only need one for serialization and one for deserialization. In this change, I create and cache these for reuse, bringing it down from 4000 in one test to just 4.
1 parent ac64578 commit 8bbb138

File tree

1 file changed

+58
-6
lines changed

1 file changed

+58
-6
lines changed

src/StreamJsonRpc/MessagePackFormatter.cs

Lines changed: 58 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,10 @@ public class MessagePackFormatter : IJsonRpcMessageFormatter, IJsonRpcInstanceCo
8787

8888
private readonly PipeFormatterResolver pipeFormatterResolver;
8989

90+
private readonly ToStringHelper serializationToStringHelper = new ToStringHelper();
91+
92+
private readonly ToStringHelper deserializationToStringHelper = new ToStringHelper();
93+
9094
/// <summary>
9195
/// Backing field for the <see cref="MultiplexingStream"/> property.
9296
/// </summary>
@@ -286,7 +290,15 @@ public JsonRpcMessage Deserialize(ReadOnlySequence<byte> contentBuffer)
286290
JsonRpcMessage message = MessagePackSerializer.Deserialize<JsonRpcMessage>(contentBuffer, this.messageSerializationOptions);
287291

288292
IJsonRpcTracingCallbacks? tracingCallbacks = this.rpc;
289-
tracingCallbacks?.OnMessageDeserialized(message, new ToStringHelper(contentBuffer, this.messageSerializationOptions));
293+
this.deserializationToStringHelper.Activate(contentBuffer, this.messageSerializationOptions);
294+
try
295+
{
296+
tracingCallbacks?.OnMessageDeserialized(message, this.deserializationToStringHelper);
297+
}
298+
finally
299+
{
300+
this.deserializationToStringHelper.Deactivate();
301+
}
290302

291303
return message;
292304
}
@@ -323,7 +335,15 @@ public void Serialize(IBufferWriter<byte> contentBuffer, JsonRpcMessage message)
323335
void IJsonRpcFormatterTracingCallbacks.OnSerializationComplete(JsonRpcMessage message, ReadOnlySequence<byte> encodedMessage)
324336
{
325337
IJsonRpcTracingCallbacks? tracingCallbacks = this.rpc;
326-
tracingCallbacks?.OnMessageSerialized(message, new ToStringHelper(encodedMessage, this.messageSerializationOptions));
338+
this.serializationToStringHelper.Activate(encodedMessage, this.messageSerializationOptions);
339+
try
340+
{
341+
tracingCallbacks?.OnMessageSerialized(message, this.serializationToStringHelper);
342+
}
343+
finally
344+
{
345+
this.serializationToStringHelper.Deactivate();
346+
}
327347
}
328348

329349
/// <inheritdoc/>
@@ -743,18 +763,50 @@ public object Convert(object value, TypeCode typeCode)
743763
public ulong ToUInt64(object value) => ((RawMessagePack)value).Deserialize<ulong>(this.options);
744764
}
745765

766+
/// <summary>
767+
/// A recyclable object that can serialize a message to JSON on demand.
768+
/// </summary>
769+
/// <remarks>
770+
/// In perf traces, creation of this object used to show up as one of the most allocated objects.
771+
/// It is used even when tracing isn't active. So we changed its design it to be reused,
772+
/// since its lifetime is only required during a synchronous call to a trace API.
773+
/// </remarks>
746774
private class ToStringHelper
747775
{
748-
private readonly ReadOnlySequence<byte> encodedMessage;
749-
private readonly MessagePackSerializerOptions options;
776+
private ReadOnlySequence<byte>? encodedMessage;
777+
private MessagePackSerializerOptions? options;
778+
private string? jsonString;
750779

751-
internal ToStringHelper(ReadOnlySequence<byte> encodedMessage, MessagePackSerializerOptions options)
780+
public override string ToString()
781+
{
782+
Verify.Operation(this.encodedMessage.HasValue, "This object has not been activated. It may have already been recycled.");
783+
784+
if (this.jsonString is null)
785+
{
786+
this.jsonString = MessagePackSerializer.ConvertToJson(this.encodedMessage.Value, this.options);
787+
}
788+
789+
return this.jsonString;
790+
}
791+
792+
/// <summary>
793+
/// Initializes this object to represent a message.
794+
/// </summary>
795+
internal void Activate(ReadOnlySequence<byte> encodedMessage, MessagePackSerializerOptions options)
752796
{
753797
this.encodedMessage = encodedMessage;
754798
this.options = options;
755799
}
756800

757-
public override string ToString() => MessagePackSerializer.ConvertToJson(this.encodedMessage, this.options);
801+
/// <summary>
802+
/// Cleans out this object to release memory and ensure <see cref="ToString"/> throws if someone uses it after deactivation.
803+
/// </summary>
804+
internal void Deactivate()
805+
{
806+
this.encodedMessage = null;
807+
this.options = null;
808+
this.jsonString = null;
809+
}
758810
}
759811

760812
private class RequestIdFormatter : IMessagePackFormatter<RequestId>

0 commit comments

Comments
 (0)