[STJ] Replace module-wide SkipLocalsInit with per-method [SkipLocalsInit]#128056
[STJ] Replace module-wide SkipLocalsInit with per-method [SkipLocalsInit]#128056EgorBo wants to merge 1 commit into
Conversation
…nit] Experiment to see impact on asmdiffs. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Tagging subscribers to this area: @dotnet/area-system-text-json |
There was a problem hiding this comment.
Pull request overview
This PR changes System.Text.Json’s handling of SkipLocalsInit from a project/module-wide setting to targeted per-method [SkipLocalsInit] attributes, primarily on hot paths that use stackalloc. It also adds a downlevel polyfill for SkipLocalsInitAttribute so the attribute can be used when targeting frameworks that don’t provide it.
Changes:
- Set
<SkipLocalsInit>false</SkipLocalsInit>inSystem.Text.Json.csprojand apply[SkipLocalsInit]to specific methods across writer/reader/DOM/converters. - Add an internal polyfill
System.Runtime.CompilerServices.SkipLocalsInitAttributefor non-.NETCoreApp TFMs. - Add
using System.Runtime.CompilerServices;where needed.
Reviewed changes
Copilot reviewed 48 out of 48 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/libraries/Common/src/Polyfills/SkipLocalsInitAttributePolyfill.cs | Adds downlevel SkipLocalsInitAttribute polyfill for non-.NETCoreApp targets. |
| src/libraries/System.Text.Json/src/System.Text.Json.csproj | Disables project-wide SkipLocalsInit and wires in the polyfill for non-.NETCoreApp TFMs. |
| src/libraries/System.Text.Json/src/System/Text/Json/Document/JsonDocument.TryGetProperty.cs | Applies [SkipLocalsInit] to stackalloc-heavy property lookup paths. |
| src/libraries/System.Text.Json/src/System/Text/Json/Document/JsonDocument.cs | Applies [SkipLocalsInit] to stackalloc-heavy string comparison helper. |
| src/libraries/System.Text.Json/src/System/Text/Json/JsonEncodedText.cs | Applies [SkipLocalsInit] to transcoding/encoding helper using stackalloc. |
| src/libraries/System.Text.Json/src/System/Text/Json/JsonHelpers.Escaping.cs | Applies [SkipLocalsInit] to escaping helpers using stackalloc. |
| src/libraries/System.Text.Json/src/System/Text/Json/JsonHelpers.cs | Applies [SkipLocalsInit] to helper using stackalloc buffers. |
| src/libraries/System.Text.Json/src/System/Text/Json/Nodes/JsonArray.cs | Applies [SkipLocalsInit] to JSON-path building logic using stackalloc. |
| src/libraries/System.Text.Json/src/System/Text/Json/Nodes/JsonNode.cs | Applies [SkipLocalsInit] to GetPath() which uses ValueStringBuilder(stackalloc ...). |
| src/libraries/System.Text.Json/src/System/Text/Json/Reader/JsonReaderHelper.Unescaping.cs | Applies [SkipLocalsInit] to unescaping/transcoding helpers using stackalloc and pooling. |
| src/libraries/System.Text.Json/src/System/Text/Json/Reader/JsonReaderHelper.cs | Applies [SkipLocalsInit] to escaped DateTime/Guid parsing helpers using stackalloc. |
| src/libraries/System.Text.Json/src/System/Text/Json/Reader/Utf8JsonReader.MultiSegment.cs | Applies [SkipLocalsInit] to multi-segment literal validation path using stackalloc. |
| src/libraries/System.Text.Json/src/System/Text/Json/Reader/Utf8JsonReader.TryGet.cs | Applies [SkipLocalsInit] to string-copy and DateTime/Guid TryGet paths using stackalloc. |
| src/libraries/System.Text.Json/src/System/Text/Json/Reader/Utf8JsonReader.cs | Applies [SkipLocalsInit] to ValueTextEquals(ReadOnlySpan<char>) which uses stackalloc/pooling. |
| src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Value/CharConverter.cs | Applies [SkipLocalsInit] to stackalloc-based formatting/parsing path. |
| src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Value/DateOnlyConverter.cs | Applies [SkipLocalsInit] to stackalloc-based parsing/formatting helpers. |
| src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Value/EnumConverter.cs | Applies [SkipLocalsInit] to stackalloc/pooling enum formatting/parsing helpers. |
| src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Value/HalfConverter.cs | Applies [SkipLocalsInit] to stackalloc-based floating-point formatting/parsing helpers. |
| src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Value/Int128Converter.cs | Applies [SkipLocalsInit] to Int128 parsing/formatting helpers (stackalloc/pooling). |
| src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Value/TimeOnlyConverter.cs | Applies [SkipLocalsInit] to stackalloc-based parsing/formatting helpers. |
| src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Value/TimeSpanConverter.cs | Applies [SkipLocalsInit] to stackalloc-based parsing/formatting helpers. |
| src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Value/UInt128Converter.cs | Applies [SkipLocalsInit] to UInt128 parsing/formatting helpers (stackalloc/pooling). |
| src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Value/VersionConverter.cs | Applies [SkipLocalsInit] to stackalloc/pooling version parsing helper. |
| src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Read.String.cs | Applies [SkipLocalsInit] to span/string reading paths using stackalloc/pooling. |
| src/libraries/System.Text.Json/src/System/Text/Json/ThrowHelper.Serialization.cs | Applies [SkipLocalsInit] to truncate helper using stackalloc. |
| src/libraries/System.Text.Json/src/System/Text/Json/Writer/JsonWriterHelper.Date.cs | Applies [SkipLocalsInit] to date formatting helpers using stackalloc. |
| src/libraries/System.Text.Json/src/System/Text/Json/Writer/JsonWriterHelper.cs | Applies [SkipLocalsInit] to transcoding/escaping helpers using stackalloc/pooling. |
| src/libraries/System.Text.Json/src/System/Text/Json/Writer/Utf8JsonWriter.cs | Applies [SkipLocalsInit] to hot writer paths using stackalloc buffers. |
| src/libraries/System.Text.Json/src/System/Text/Json/Writer/Utf8JsonWriter.WriteProperties.Bytes.cs | Applies [SkipLocalsInit] to stackalloc-based property-name writing. |
| src/libraries/System.Text.Json/src/System/Text/Json/Writer/Utf8JsonWriter.WriteProperties.DateTime.cs | Applies [SkipLocalsInit] to stackalloc-based DateTime property-name/value escaping/writing. |
| src/libraries/System.Text.Json/src/System/Text/Json/Writer/Utf8JsonWriter.WriteProperties.DateTimeOffset.cs | Applies [SkipLocalsInit] to stackalloc-based DateTimeOffset property-name/value escaping/writing. |
| src/libraries/System.Text.Json/src/System/Text/Json/Writer/Utf8JsonWriter.WriteProperties.Decimal.cs | Applies [SkipLocalsInit] to stackalloc-based decimal property-name/value escaping/writing. |
| src/libraries/System.Text.Json/src/System/Text/Json/Writer/Utf8JsonWriter.WriteProperties.Double.cs | Applies [SkipLocalsInit] to stackalloc-based double property-name/value escaping/writing. |
| src/libraries/System.Text.Json/src/System/Text/Json/Writer/Utf8JsonWriter.WriteProperties.Float.cs | Applies [SkipLocalsInit] to stackalloc-based float property-name/value escaping/writing. |
| src/libraries/System.Text.Json/src/System/Text/Json/Writer/Utf8JsonWriter.WriteProperties.FormattedNumber.cs | Applies [SkipLocalsInit] to stackalloc-based formatted-number property-name writing. |
| src/libraries/System.Text.Json/src/System/Text/Json/Writer/Utf8JsonWriter.WriteProperties.Guid.cs | Applies [SkipLocalsInit] to stackalloc-based Guid property-name/value escaping/writing. |
| src/libraries/System.Text.Json/src/System/Text/Json/Writer/Utf8JsonWriter.WriteProperties.Literal.cs | Applies [SkipLocalsInit] to stackalloc-based literal property-name/value escaping/writing. |
| src/libraries/System.Text.Json/src/System/Text/Json/Writer/Utf8JsonWriter.WriteProperties.SignedNumber.cs | Applies [SkipLocalsInit] to stackalloc-based signed-number property-name/value escaping/writing. |
| src/libraries/System.Text.Json/src/System/Text/Json/Writer/Utf8JsonWriter.WriteProperties.String.cs | Applies [SkipLocalsInit] to stackalloc-based string property-name/value escaping/writing. |
| src/libraries/System.Text.Json/src/System/Text/Json/Writer/Utf8JsonWriter.WriteProperties.UnsignedNumber.cs | Applies [SkipLocalsInit] to stackalloc-based unsigned-number property-name/value escaping/writing. |
| src/libraries/System.Text.Json/src/System/Text/Json/Writer/Utf8JsonWriter.WriteValues.Decimal.cs | Applies [SkipLocalsInit] to stackalloc-based decimal value writing. |
| src/libraries/System.Text.Json/src/System/Text/Json/Writer/Utf8JsonWriter.WriteValues.Double.cs | Applies [SkipLocalsInit] to stackalloc-based double value writing. |
| src/libraries/System.Text.Json/src/System/Text/Json/Writer/Utf8JsonWriter.WriteValues.Float.cs | Applies [SkipLocalsInit] to stackalloc-based float value writing. |
| src/libraries/System.Text.Json/src/System/Text/Json/Writer/Utf8JsonWriter.WriteValues.Raw.cs | Applies [SkipLocalsInit] to raw-value transcoding path; also includes an incidental header encoding/whitespace change. |
| src/libraries/System.Text.Json/src/System/Text/Json/Writer/Utf8JsonWriter.WriteValues.SignedNumber.cs | Applies [SkipLocalsInit] to stackalloc-based signed-number value writing. |
| src/libraries/System.Text.Json/src/System/Text/Json/Writer/Utf8JsonWriter.WriteValues.String.cs | Applies [SkipLocalsInit] to stackalloc-based string escaping/writing. |
| src/libraries/System.Text.Json/src/System/Text/Json/Writer/Utf8JsonWriter.WriteValues.StringSegment.cs | Applies [SkipLocalsInit] to stackalloc-based segmented string/base64 writing. |
| src/libraries/System.Text.Json/src/System/Text/Json/Writer/Utf8JsonWriter.WriteValues.UnsignedNumber.cs | Applies [SkipLocalsInit] to stackalloc-based unsigned-number value writing. |
Comments suppressed due to low confidence (2)
src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Value/Int128Converter.cs:67
- If Int128.TryParse fails, this method throws before returning the rented ArrayPool buffer, leaking it. Wrap the parse / throw path in a try/finally (or return the buffer before throwing) so rentedBuffer is always returned, including on FormatException paths.
int bufferLength = reader.ValueLength;
byte[]? rentedBuffer = null;
Span<byte> buffer = bufferLength <= JsonConstants.StackallocByteThreshold
? stackalloc byte[JsonConstants.StackallocByteThreshold]
: (rentedBuffer = ArrayPool<byte>.Shared.Rent(bufferLength));
int written = reader.CopyValue(buffer);
if (!Int128.TryParse(buffer.Slice(0, written), CultureInfo.InvariantCulture, out Int128 result))
{
ThrowHelper.ThrowFormatException(NumericType.Int128);
}
if (rentedBuffer != null)
{
ArrayPool<byte>.Shared.Return(rentedBuffer);
}
src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Value/UInt128Converter.cs:67
- If UInt128.TryParse fails, this method throws before returning the rented ArrayPool buffer, leaking it. Ensure rentedBuffer is returned in all paths (e.g., via try/finally) so invalid JSON inputs don't permanently consume pooled buffers.
int bufferLength = reader.ValueLength;
byte[]? rentedBuffer = null;
Span<byte> buffer = bufferLength <= JsonConstants.StackallocByteThreshold
? stackalloc byte[JsonConstants.StackallocByteThreshold]
: (rentedBuffer = ArrayPool<byte>.Shared.Rent(bufferLength));
int written = reader.CopyValue(buffer);
if (!UInt128.TryParse(buffer.Slice(0, written), CultureInfo.InvariantCulture, out UInt128 result))
{
ThrowHelper.ThrowFormatException(NumericType.UInt128);
}
if (rentedBuffer != null)
{
ArrayPool<byte>.Shared.Return(rentedBuffer);
}
| // Licensed to the .NET Foundation under one or more agreements. | ||
| // The .NET Foundation licenses this file to you under the MIT license. | ||
|
|
experiment to see impact on asmdiffs
Note
This PR description was authored by GitHub Copilot.