Skip to content

Commit

Permalink
.Net: Avoid creating new JsonSerializerOptions per Serialize/Deserial…
Browse files Browse the repository at this point in the history
…ize (#3509)

### Motivation and Context

JsonSerializerOptions is unexpectedly expensive to create and throw away
as it gets used as a cache for state gathered as part of the
Serialize/Deserialize call.

### Contribution Checklist

<!-- Before submitting this PR, please make sure: -->

- [x] The code builds clean without any errors or warnings
- [x] The PR follows the [SK Contribution
Guidelines](https://github.com/microsoft/semantic-kernel/blob/main/CONTRIBUTING.md)
and the [pre-submission formatting
script](https://github.com/microsoft/semantic-kernel/blob/main/CONTRIBUTING.md#development-scripts)
raises no violations
- [x] All unit tests pass, and I have added new tests where possible
- [x] I didn't break anyone 😄
  • Loading branch information
stephentoub committed Nov 15, 2023
1 parent fb6af41 commit 55cb52e
Show file tree
Hide file tree
Showing 4 changed files with 33 additions and 21 deletions.
18 changes: 8 additions & 10 deletions dotnet/src/Experimental/Orchestration.Flow/FlowSerializer.cs
Expand Up @@ -16,6 +16,13 @@ namespace Microsoft.SemanticKernel.Experimental.Orchestration;
/// </summary>
public static class FlowSerializer
{
/// <summary>Options for <see cref="DeserializeFromJson"/>.</summary>
private static readonly JsonSerializerOptions s_deserializeOptions = new()
{
PropertyNameCaseInsensitive = true,
Converters = { new JsonStringEnumConverter(JsonNamingPolicy.CamelCase) }
};

/// <summary>
/// Deserialize flow from yaml
/// </summary>
Expand All @@ -39,17 +46,8 @@ public static Flow DeserializeFromYaml(string yaml)
/// <returns>the <see cref="Flow"/> instance</returns>
public static Flow? DeserializeFromJson(string json)
{
var options = new JsonSerializerOptions
{
PropertyNameCaseInsensitive = true,
Converters = { new JsonStringEnumConverter(JsonNamingPolicy.CamelCase) }
};

var flow = JsonSerializer.Deserialize<FlowModel>(json, options);
if (flow is null)
{
var flow = JsonSerializer.Deserialize<FlowModel>(json, s_deserializeOptions) ??
throw new JsonException("Failed to deserialize flow");
}

return UpCast(flow);
}
Expand Down
8 changes: 6 additions & 2 deletions dotnet/src/Functions/Functions.Grpc/GrpcOperationRunner.cs
Expand Up @@ -24,6 +24,10 @@ namespace Microsoft.SemanticKernel.Functions.Grpc;
/// </summary>
internal sealed class GrpcOperationRunner
{
/// <summary>Serialization options that use a camel casing naming policy.</summary>
private static readonly JsonSerializerOptions s_camelCaseOptions = new() { PropertyNamingPolicy = JsonNamingPolicy.CamelCase };
/// <summary>Deserialization options that use case-insensitive property names.</summary>
private static readonly JsonSerializerOptions s_propertyCaseInsensitiveOptions = new() { PropertyNameCaseInsensitive = true };
/// <summary>
/// An instance of the HttpClient class.
/// </summary>
Expand Down Expand Up @@ -87,7 +91,7 @@ public async Task<JsonObject> RunAsync(GrpcOperation operation, IDictionary<stri
/// <returns>The converted response.</returns>
private static JsonObject ConvertResponse(object response, Type responseType)
{
var content = JsonSerializer.Serialize(response, responseType, new JsonSerializerOptions { PropertyNamingPolicy = JsonNamingPolicy.CamelCase });
var content = JsonSerializer.Serialize(response, responseType, s_camelCaseOptions);

//First iteration allowing to associate additional metadata with the returned content.
var result = new JsonObject();
Expand Down Expand Up @@ -159,7 +163,7 @@ private object GenerateOperationRequest(GrpcOperation operation, Type type, IDic
}

//Deserializing JSON payload to gRPC request message
var instance = JsonSerializer.Deserialize(payload, type, new JsonSerializerOptions { PropertyNameCaseInsensitive = true });
var instance = JsonSerializer.Deserialize(payload, type, s_propertyCaseInsensitiveOptions);
if (instance == null)
{
throw new SKException($"Impossible to create gRPC request message for the '{operation.Name}' gRPC operation.");
Expand Down
17 changes: 10 additions & 7 deletions dotnet/src/Planners/Planners.Core/Action/ActionPlanner.cs
Expand Up @@ -40,6 +40,15 @@ public sealed class ActionPlanner : IActionPlanner
/// </summary>
private static readonly Regex s_planRegex = new("^[^{}]*(((?'Open'{)[^{}]*)+((?'Close-Open'})[^{}]*)+)*(?(Open)(?!))", RegexOptions.Singleline | RegexOptions.Compiled);

/// <summary>Deserialization options for use with <see cref="ActionPlanResponse"/>.</summary>
private static readonly JsonSerializerOptions s_actionPlayResponseOptions = new()
{
AllowTrailingCommas = true,
DictionaryKeyPolicy = null,
DefaultIgnoreCondition = JsonIgnoreCondition.Never,
PropertyNameCaseInsensitive = true,
};

// Planner semantic function
private readonly ISKFunction _plannerFunction;

Expand Down Expand Up @@ -261,13 +270,7 @@ No parameters.
string planJson = $"{{{match.Groups["Close"]}}}";
try
{
return JsonSerializer.Deserialize<ActionPlanResponse?>(planJson, new JsonSerializerOptions
{
AllowTrailingCommas = true,
DictionaryKeyPolicy = null,
DefaultIgnoreCondition = JsonIgnoreCondition.Never,
PropertyNameCaseInsensitive = true,
});
return JsonSerializer.Deserialize<ActionPlanResponse?>(planJson, s_actionPlayResponseOptions);
}
catch (Exception e)
{
Expand Down
11 changes: 9 additions & 2 deletions dotnet/src/SemanticKernel.Core/Planning/Plan.cs
Expand Up @@ -167,7 +167,7 @@ public Plan(ISKFunction function)
/// <remarks>If Context is not supplied, plan will not be able to execute.</remarks>
public static Plan FromJson(string json, IReadOnlyFunctionCollection? functions = null, bool requireFunctions = true)
{
var plan = JsonSerializer.Deserialize<Plan>(json, new JsonSerializerOptions { IncludeFields = true }) ?? new Plan(string.Empty);
var plan = JsonSerializer.Deserialize<Plan>(json, s_includeFieldsOptions) ?? new Plan(string.Empty);

if (functions != null)
{
Expand All @@ -184,7 +184,9 @@ public static Plan FromJson(string json, IReadOnlyFunctionCollection? functions
/// <returns>Plan serialized using JSON format</returns>
public string ToJson(bool indented = false)
{
return JsonSerializer.Serialize(this, new JsonSerializerOptions { WriteIndented = indented });
return indented ?
JsonSerializer.Serialize(this, s_writeIndentedOptions) :
JsonSerializer.Serialize(this);
}

/// <summary>
Expand Down Expand Up @@ -680,6 +682,11 @@ private void SetFunction(ISKFunction function)

private static string GetRandomPlanName() => "plan" + Guid.NewGuid().ToString("N");

/// <summary>Deserialization options for including fields.</summary>
private static readonly JsonSerializerOptions s_includeFieldsOptions = new() { IncludeFields = true };
/// <summary>Serialization options for writing indented.</summary>
private static readonly JsonSerializerOptions s_writeIndentedOptions = new() { WriteIndented = true };

private ISKFunction? Function { get; set; }

private readonly List<Plan> _steps = new();
Expand Down

0 comments on commit 55cb52e

Please sign in to comment.