-
Notifications
You must be signed in to change notification settings - Fork 1k
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.
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
Add StringBuilder pooling in NewtonsoftJsonSerializer #4929
Merged
Aaronontheweb
merged 12 commits into
akkadotnet:dev
from
to11mtm:newtonsoftjsonserializer-pooled-stringbuilder
Dec 14, 2021
Merged
Changes from all commits
Commits
Show all changes
12 commits
Select commit
Hold shift + click to select a range
0d5b076
Add StringBuilder pooling in NewtonsoftJsonSerializer
to11mtm 7c72de5
Fix CreateInternalSettings to properly wire everything up and be hard…
to11mtm 66c87f4
don't make two settings.
to11mtm 379a51f
Fixes for thread safety, fix using scopes.
to11mtm ea00658
api approvals, chars are not bytes so lets not confuse ourselves in H…
to11mtm 2727bfa
fix api approval
to11mtm 9edbbf6
Merge branch 'dev' into newtonsoftjsonserializer-pooled-stringbuilder
Aaronontheweb edbff06
Merge branch 'dev' into newtonsoftjsonserializer-pooled-stringbuilder
to11mtm e0a83de
Merge branch 'dev' into newtonsoftjsonserializer-pooled-stringbuilder
to11mtm b343de8
Merge branch 'dev' into newtonsoftjsonserializer-pooled-stringbuilder
Aaronontheweb e07aa41
Merge branch 'dev' into newtonsoftjsonserializer-pooled-stringbuilder
Aaronontheweb 3677850
Merge branch 'dev' into newtonsoftjsonserializer-pooled-stringbuilder
Aaronontheweb File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -8,12 +8,14 @@ | |
using System; | ||
using System.Collections.Generic; | ||
using System.Globalization; | ||
using System.IO; | ||
using System.Linq; | ||
using System.Reflection; | ||
using System.Text; | ||
using Akka.Actor; | ||
using Akka.Configuration; | ||
using Akka.Util; | ||
using Microsoft.Extensions.ObjectPool; | ||
using Newtonsoft.Json; | ||
using Newtonsoft.Json.Converters; | ||
using Newtonsoft.Json.Linq; | ||
|
@@ -32,7 +34,10 @@ public sealed class NewtonSoftJsonSerializerSettings | |
public static readonly NewtonSoftJsonSerializerSettings Default = new NewtonSoftJsonSerializerSettings( | ||
encodeTypeNames: true, | ||
preserveObjectReferences: true, | ||
converters: Enumerable.Empty<Type>()); | ||
converters: Enumerable.Empty<Type>(), | ||
usePooledStringBuilder:true, | ||
stringBuilderMinSize:2048, | ||
stringBuilderMaxSize:32768); | ||
|
||
/// <summary> | ||
/// Creates a new instance of the <see cref="NewtonSoftJsonSerializerSettings"/> based on a provided <paramref name="config"/>. | ||
|
@@ -52,8 +57,15 @@ public static NewtonSoftJsonSerializerSettings Create(Config config) | |
|
||
return new NewtonSoftJsonSerializerSettings( | ||
encodeTypeNames: config.GetBoolean("encode-type-names", true), | ||
preserveObjectReferences: config.GetBoolean("preserve-object-references", true), | ||
converters: GetConverterTypes(config)); | ||
preserveObjectReferences: config.GetBoolean( | ||
"preserve-object-references", true), | ||
converters: GetConverterTypes(config), | ||
usePooledStringBuilder: config.GetBoolean("use-pooled-string-builder", true), | ||
stringBuilderMinSize:config.GetInt("pooled-string-builder-minsize", 2048), | ||
stringBuilderMaxSize: | ||
config.GetInt("pooled-string-builder-maxsize", | ||
32768) | ||
); | ||
} | ||
|
||
private static IEnumerable<Type> GetConverterTypes(Config config) | ||
|
@@ -89,21 +101,40 @@ private static IEnumerable<Type> GetConverterTypes(Config config) | |
/// Converters must inherit from <see cref="JsonConverter"/> class and implement a default constructor. | ||
/// </summary> | ||
public IEnumerable<Type> Converters { get; } | ||
|
||
/// <summary> | ||
/// The Starting size used for Pooled StringBuilders, if <see cref="UsePooledStringBuilder"/> is -true- | ||
/// </summary> | ||
public int StringBuilderMinSize { get; } | ||
/// <summary> | ||
/// The Max Retained size for Pooled StringBuilders, if <see cref="UsePooledStringBuilder"/> is -true- | ||
/// </summary> | ||
public int StringBuilderMaxSize { get; } | ||
/// <summary> | ||
/// If -true-, Stringbuilders are pooled and reused for serialization to lower memory pressure. | ||
/// </summary> | ||
public bool UsePooledStringBuilder { get; } | ||
|
||
/// <summary> | ||
/// Creates a new instance of the <see cref="NewtonSoftJsonSerializerSettings"/>. | ||
/// </summary> | ||
/// <param name="encodeTypeNames">Determines if a special `$type` field should be emitted into serialized JSON. Must be true if corresponding serializer is used as default.</param> | ||
/// <param name="preserveObjectReferences">Determines if object references should be tracked within serialized object graph. Must be true if corresponding serialize is used as default.</param> | ||
/// <param name="converters">A list of types implementing a <see cref="JsonConverter"/> to support custom types serialization.</param> | ||
public NewtonSoftJsonSerializerSettings(bool encodeTypeNames, bool preserveObjectReferences, IEnumerable<Type> converters) | ||
/// <param name="usePooledStringBuilder">Determines if string builders will be used from a pool to lower memory usage</param> | ||
/// <param name="stringBuilderMinSize">Starting size used for pooled string builders if enabled</param> | ||
/// <param name="stringBuilderMaxSize">Max retained size used for pooled string builders if enabled</param> | ||
public NewtonSoftJsonSerializerSettings(bool encodeTypeNames, bool preserveObjectReferences, IEnumerable<Type> converters, bool usePooledStringBuilder, int stringBuilderMinSize, int stringBuilderMaxSize) | ||
{ | ||
if (converters == null) | ||
throw new ArgumentNullException(nameof(converters), $"{nameof(NewtonSoftJsonSerializerSettings)} requires a sequence of converters."); | ||
|
||
EncodeTypeNames = encodeTypeNames; | ||
PreserveObjectReferences = preserveObjectReferences; | ||
Converters = converters; | ||
UsePooledStringBuilder = usePooledStringBuilder; | ||
StringBuilderMinSize = stringBuilderMinSize; | ||
StringBuilderMaxSize = stringBuilderMaxSize; | ||
} | ||
} | ||
|
||
|
@@ -114,7 +145,8 @@ public NewtonSoftJsonSerializerSettings(bool encodeTypeNames, bool preserveObjec | |
public class NewtonSoftJsonSerializer : Serializer | ||
{ | ||
private readonly JsonSerializer _serializer; | ||
|
||
|
||
private readonly ObjectPool<StringBuilder> _sbPool; | ||
/// <summary> | ||
/// TBD | ||
/// </summary> | ||
|
@@ -138,11 +170,15 @@ public NewtonSoftJsonSerializer(ExtendedActorSystem system, Config config) | |
: this(system, NewtonSoftJsonSerializerSettings.Create(config)) | ||
{ | ||
} | ||
|
||
|
||
|
||
public NewtonSoftJsonSerializer(ExtendedActorSystem system, NewtonSoftJsonSerializerSettings settings) | ||
: base(system) | ||
{ | ||
if (settings.UsePooledStringBuilder) | ||
{ | ||
_sbPool = new DefaultObjectPoolProvider() | ||
.CreateStringBuilderPool(settings.StringBuilderMinSize,settings.StringBuilderMaxSize); | ||
} | ||
Settings = new JsonSerializerSettings | ||
{ | ||
PreserveReferencesHandling = settings.PreserveObjectReferences | ||
|
@@ -183,6 +219,8 @@ public NewtonSoftJsonSerializer(ExtendedActorSystem system, NewtonSoftJsonSerial | |
_serializer = JsonSerializer.Create(Settings); | ||
} | ||
|
||
|
||
|
||
private static JsonConverter CreateConverter(Type converterType, ExtendedActorSystem actorSystem) | ||
{ | ||
var ctor = converterType.GetConstructors() | ||
|
@@ -228,12 +266,56 @@ protected override JsonProperty CreateProperty(MemberInfo member, MemberSerializ | |
/// <param name="obj">The object to serialize </param> | ||
/// <returns>A byte array containing the serialized object</returns> | ||
public override byte[] ToBinary(object obj) | ||
{ | ||
if (_sbPool != null) | ||
{ | ||
return toBinary_PooledBuilder(obj); | ||
} | ||
else | ||
{ | ||
return toBinary_NewBuilder(obj); | ||
} | ||
|
||
} | ||
|
||
private byte[] toBinary_NewBuilder(object obj) | ||
{ | ||
string data = JsonConvert.SerializeObject(obj, Formatting.None, Settings); | ||
byte[] bytes = Encoding.UTF8.GetBytes(data); | ||
return bytes; | ||
} | ||
|
||
private byte[] toBinary_PooledBuilder(object obj) | ||
{ | ||
//Don't try to opt with | ||
//StringBuilder sb = _sbPool.Get() | ||
//Or removing null check | ||
//Both are necessary to avoid leaking on thread aborts etc | ||
StringBuilder sb = null; | ||
try | ||
{ | ||
sb = _sbPool.Get(); | ||
|
||
using (var tw = new StringWriter(sb, CultureInfo.InvariantCulture)) | ||
{ | ||
var ser = JsonSerializer.CreateDefault(Settings); | ||
ser.Formatting = Formatting.None; | ||
using (var jw = new JsonTextWriter(tw)) | ||
{ | ||
ser.Serialize(jw, obj); | ||
} | ||
return Encoding.UTF8.GetBytes(tw.ToString()); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. LGTM |
||
} | ||
} | ||
finally | ||
{ | ||
if (sb != null) | ||
{ | ||
_sbPool.Return(sb); | ||
} | ||
} | ||
} | ||
|
||
/// <summary> | ||
/// Deserializes a byte array into an object of type <paramref name="type"/>. | ||
/// </summary> | ||
|
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
At some point this will get folded into the core runtime when we move on from .NET Standard 2.0, IIRC