Skip to content

PERF: Reduce boxing allocations in GetScalarValue #2938

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

Open
Youssef Fahmy (Youssef1313) wants to merge 5 commits into
base: main
Choose a base branch
from
Open

PERF: Reduce boxing allocations in GetScalarValue #2938

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
73d9648
PERF: Reduce boxing allocations in GetScalarValue
Youssef1313 Jul 10, 2026
9bdea50
Reduce unnecessary allocations in JSON parsing
Youssef1313 Jul 10, 2026
9e7ad1c
Simplify
Youssef1313 Jul 14, 2026
512cd81
Avoid unnecessary ToDictionary
Youssef1313 Jul 14, 2026
f120c66
chore: typo fix
baywet Jul 14, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
57 changes: 56 additions & 1 deletion src/Microsoft.OpenApi/Reader/JsonNodeHelper.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT license.

using System;
using System.Collections.Generic;
using System.Globalization;
using System.Linq;
using System.Text.Json;
using System.Text.Json.Nodes;

namespace Microsoft.OpenApi.Reader
Expand Down Expand Up @@ -159,9 +160,63 @@ public static Dictionary<string, HashSet<T>> CreateArrayMap<T>(this JsonNode? no
{
var scalarNode = node is JsonValue value ? value : throw new OpenApiException("Expected scalar value.");

// It's much more efficient to call scalarNode.TryGetValue<string> than to call scalarNode.GetValue<object>() and then convert to string.
// When asking for "object" type, if scalarNode is JsonValueOfElement (internal type in STJ), we will get back
// a boxed JsonElement (paying the cost of unnecessary boxing allocation), and we then call JsonElement.ToString.
// So, we first check if we can get the string value directly, and only if that fails, we fallback to the expensive code.
Comment on lines +163 to +166

@baywet Vincent Biret (baywet) Jul 13, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think that one reason why we had to do that was to avoid mangling like default, numbers with formats, etc... While I do appreciate the performance improvements, I don't want it to come at the price of a regression.

@Youssef1313 Youssef Fahmy (Youssef1313) Jul 14, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

TryGetValue<string> is never going to throw or do something strange. It will return true if the node really represents a string. For example, if you have any numbers in JSON like "maximum": 10, TryGetValue<string> will return false because the node doesn't represent a string. And then we fallback to the original implementation.

I done it this way explicitly because I saw that some numerics are read raw as strings that way.

@baywet Vincent Biret (baywet) Jul 14, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is what I'm referring to c17a87e

I think the object convert was introduced to avoid formatting issues for defaults and other properties when using types like dates and such.

if (scalarNode.TryGetValue<string>(out var stringValue))
{
return stringValue;
}

return Convert.ToString(scalarNode.GetValue<object>(), CultureInfo.InvariantCulture);
}

public static bool GetScalarBoolValue(this JsonNode? node)
{
var scalarNode = node is JsonValue value ? value : throw new OpenApiException("Expected scalar value.");
return scalarNode.GetValue<bool>();
}
Comment on lines +175 to +179

public static int GetScalarIntValue(this JsonNode? node)
{
var scalarNode = node is JsonValue value && value.GetValueKind() == JsonValueKind.Number ? value : throw new OpenApiException("Expected numeric scalar value.");
Comment thread
Youssef1313 marked this conversation as resolved.

if (scalarNode.TryGetValue<int>(out var intValue))
{
return intValue;
}
else if (scalarNode.TryGetValue<long>(out var longValue))
{
return (int)longValue;
}
else if (scalarNode.TryGetValue<decimal>(out var decimalValue))

@baywet Vincent Biret (baywet) Jul 13, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

can we get other numeric types?

@Youssef1313 Youssef Fahmy (Youssef1313) Jul 14, 2026 •
edited
Loading

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Decimal in most cases will be coming through the YML library being used to convert YML to JSON.
I'm not sure if we can get other numeric types but I haven't seen it when trying to run the test suite. However, to reduce any risks, I kept the GetValue<object> fallback.

Edit: It's not the library. It's our code to handle YML. I tried to adjust it so that it handles numeric in a more correct way.

{
return (int)decimalValue;
}

return Convert.ToInt32(scalarNode.GetValue<object>());
}
Comment thread
Youssef1313 marked this conversation as resolved.
Comment on lines +181 to +191

public static uint GetScalarUIntValue(this JsonNode? node)
{
var scalarNode = node is JsonValue value && value.GetValueKind() == JsonValueKind.Number ? value : throw new OpenApiException("Expected numeric scalar value.");
if (scalarNode.TryGetValue<uint>(out var uintValue))
{
return uintValue;
}
else if (scalarNode.TryGetValue<ulong>(out var ulongValue))
{
return (uint)ulongValue;
}
else if (scalarNode.TryGetValue<decimal>(out var decimalValue))
{
return (uint)decimalValue;
}

return Convert.ToUInt32(scalarNode.GetValue<object>());
}
Comment thread
Youssef1313 marked this conversation as resolved.
Comment on lines +193 to +202

public static string? GetReferencePointer(this JsonObject jsonObject)
{
return jsonObject.TryGetPropertyValue("$ref", out var refNode) ? refNode?.GetScalarValue() : null;
Expand Down
40 changes: 10 additions & 30 deletions src/Microsoft.OpenApi/Reader/V2/OpenApiHeaderDeserializer.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT license.

using System.Text.Json.Nodes;
Expand Down Expand Up @@ -67,7 +67,7 @@ internal static partial class OpenApiV2Deserializer
},
{
"exclusiveMaximum",
(o, n, _, _) => GetOrCreateSchema(o).IsExclusiveMaximum = bool.Parse(n.GetScalarValue()!)
(o, n, _, _) => GetOrCreateSchema(o).IsExclusiveMaximum = n.GetScalarBoolValue()
},
{
"minimum",
Expand All @@ -76,34 +76,26 @@ internal static partial class OpenApiV2Deserializer
var min = n.GetScalarValue();
if (!string.IsNullOrEmpty(min))
{
GetOrCreateSchema(o).Minimum = min;
}
GetOrCreateSchema(o).Minimum = min;
}
}
},
{
"exclusiveMinimum",
(o, n, _, _) => GetOrCreateSchema(o).IsExclusiveMinimum = bool.Parse(n.GetScalarValue()!)
(o, n, _, _) => GetOrCreateSchema(o).IsExclusiveMinimum = n.GetScalarBoolValue()
},
{
"maxLength",
(o, n, _, _) =>
{
var maxLength = n.GetScalarValue();
if (maxLength != null)
{
GetOrCreateSchema(o).MaxLength = int.Parse(maxLength, CultureInfo.InvariantCulture);
}
GetOrCreateSchema(o).MaxLength = n.GetScalarIntValue();
}
},
{
"minLength",
(o, n, _, _) =>
{
var minLength = n.GetScalarValue();
if (minLength != null)
{
GetOrCreateSchema(o).MinLength = int.Parse(minLength, CultureInfo.InvariantCulture);
}
GetOrCreateSchema(o).MinLength = n.GetScalarIntValue();
}
},
{
Expand All @@ -114,33 +106,21 @@ internal static partial class OpenApiV2Deserializer
"maxItems",
(o, n, _, _) =>
{
var maxItems = n.GetScalarValue();
if (maxItems != null)
{
GetOrCreateSchema(o).MaxItems = int.Parse(maxItems, CultureInfo.InvariantCulture);
}
GetOrCreateSchema(o).MaxItems = n.GetScalarIntValue();
}
},
{
"minItems",
(o, n, _, _) =>
{
var minItems = n.GetScalarValue();
if (minItems != null)
{
GetOrCreateSchema(o).MinItems = int.Parse(minItems, CultureInfo.InvariantCulture);
}
GetOrCreateSchema(o).MinItems = n.GetScalarIntValue();
}
},
{
"uniqueItems",
(o, n, _, _) =>
{
var uniqueItems = n.GetScalarValue();
if (uniqueItems != null)
{
GetOrCreateSchema(o).UniqueItems = bool.Parse(uniqueItems);
}
GetOrCreateSchema(o).UniqueItems = n.GetScalarBoolValue();
}
},
{
Expand Down
8 changes: 2 additions & 6 deletions src/Microsoft.OpenApi/Reader/V2/OpenApiOperationDeserializer.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT license.

using System.Collections.Generic;
Expand Down Expand Up @@ -82,11 +82,7 @@ internal static partial class OpenApiV2Deserializer
"deprecated",
(o, n, _, _) =>
{
var deprecated = n.GetScalarValue();
if (deprecated != null)
{
o.Deprecated = bool.Parse(deprecated);
}
o.Deprecated = n.GetScalarBoolValue();
}
},
{
Expand Down
38 changes: 7 additions & 31 deletions src/Microsoft.OpenApi/Reader/V2/OpenApiParameterDeserializer.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT license.

using System.Text.Json.Nodes;
Expand Down Expand Up @@ -34,33 +34,21 @@ internal static partial class OpenApiV2Deserializer
"required",
(o, n, _, _) =>
{
var required = n.GetScalarValue();
if (required != null)
{
o.Required = bool.Parse(required);
}
o.Required = n.GetScalarBoolValue();
}
},
{
"deprecated",
(o, n, _, _) =>
{
var deprecated = n.GetScalarValue();
if (deprecated != null)
{
o.Deprecated = bool.Parse(deprecated);
}
o.Deprecated = n.GetScalarBoolValue();
}
},
{
"allowEmptyValue",
(o, n, _, _) =>
{
var allowEmptyValue = n.GetScalarValue();
if (allowEmptyValue != null)
{
o.AllowEmptyValue = bool.Parse(allowEmptyValue);
}
o.AllowEmptyValue = n.GetScalarBoolValue();
}
},
{
Expand Down Expand Up @@ -124,33 +112,21 @@ internal static partial class OpenApiV2Deserializer
"maxLength",
(o, n, _, _) =>
{
var maxLength = n.GetScalarValue();
if (maxLength != null)
{
GetOrCreateSchema(o).MaxLength = int.Parse(maxLength, CultureInfo.InvariantCulture);
}
GetOrCreateSchema(o).MaxLength = n.GetScalarIntValue();
}
},
{
"minLength",
(o, n, _, _) =>
{
var minLength = n.GetScalarValue();
if (minLength != null)
{
GetOrCreateSchema(o).MinLength = int.Parse(minLength, CultureInfo.InvariantCulture);
}
GetOrCreateSchema(o).MinLength = n.GetScalarIntValue();
}
},
{
"readOnly",
(o, n, _, _) =>
{
var readOnly = n.GetScalarValue();
if (readOnly != null)
{
GetOrCreateSchema(o).ReadOnly = bool.Parse(readOnly);
}
GetOrCreateSchema(o).ReadOnly = n.GetScalarBoolValue();
}
},
{
Expand Down
Loading
Loading