From bac3f9fb56f6378e05d7ea36041f26392bff132d Mon Sep 17 00:00:00 2001 From: Simon Cropp Date: Thu, 27 Aug 2026 18:35:30 +1000 Subject: [PATCH 01/12] Perf: vectorize hot scans, memoize repeated lookups, cut per value allocations Implements the high priority findings from the perf review in todo.md, which also records the before/after benchmark numbers quoted here. Core read/write: - ReadStringIntoBuffer skips runs of ordinary chars with a SearchValues scan rather than running every char of every string through the switch. The '\0' kept at charsUsed doubles as the scan sentinel, so the read-more-data path is unchanged. net8+ only, scalar walk retained below that. 3.6x faster. - The span WritePropertyName overload copied the name into a string so the position could hold it, allocating exactly as much as the string overload. JsonPosition now also carries the chars directly, in a buffer reused by every property at that depth. 1.7x faster, 27% less allocation. - DefaultJsonNameTable.TextEquals uses SequenceEqual. 1.9x faster. - The escape writer re-runs its vectorized scan after each escape instead of walking the remainder of the string one char at a time. Serialization: - Converter resolution is memoized for the duration of a serialize/deserialize call, rather than calling virtual CanConvert across the converter list for every value. Not cacheable on the contract, since contracts are shared across serializers with different converter lists. 2.2x serialize, 1.8x deserialize. - $type names are cached per run instead of being concatenated and re-parsed through RemoveAssemblyDetails for every object. 3.3x faster, 66% less allocation. - CalculatePropertyDetails no longer resolves the same contract twice for a property that is populated in place. - JsonObjectContract.IndexOfCreatorParameter replaces an IndexOf scan that made construction through a parameterized constructor quadratic in its parameter count. 1.6x faster on a 16 parameter record. - SetPropertyPresence does one dictionary lookup instead of two on net6+. Linq to JSON and JsonPath: - JObject.Properties iterates InnerList instead of building a LINQ Cast wrapper and boxing an enumerator. Same for CopyTo. 1.3x faster, 27% less allocation. - JTokenWriter.WriteValue(int) uses the shared BoxedPrimitives boxes. Left the narrower integer overloads alone: widening them to int would change the CLR type stored in JValue.Value, which is a behaviour change, not a perf one. - A JsonPath =~ expression caches its constructed Regex along with the timeout it was built for, instead of going through the static Regex cache per token. - JArray overrides GetItem to index its backing list directly. Supporting changes: - Adds HotPathBenchmarks covering the fixes that existing benchmarks did not, registered in Benchmark.Tests. - Fixes the ArgonTests build, which the xunit.v3 4.0.0 bump broke: the obsolete CollectionBehavior(DisableTestParallelization) is an error, so no tests ran. - Adds tests pairing the span WritePropertyName overload with Path, the only behaviour this change alters in how a value is stored and read back. - FloatParseHandlingDecimal builds its input from a decimal literal instead of casting Math.PI. The double to decimal conversion now returns full precision, which had turned the test into a check of conversion precision rather than of the XML/JSON round trip. Full suite green: 2358/2358 net10.0, 2337/2337 net48, 9/9 F#. --- src/Argon.JsonPath/BooleanQueryExpression.cs | 27 +- src/Argon/DefaultJsonNameTable.cs | 20 +- src/Argon/JsonPosition.cs | 23 +- src/Argon/JsonTextReader.cs | 32 ++ src/Argon/JsonWriter.cs | 21 +- src/Argon/Linq/JArray.cs | 5 + src/Argon/Linq/JObject.cs | 14 +- src/Argon/Linq/JTokenWriter.cs | 4 +- src/Argon/Serialization/JsonObjectContract.cs | 35 ++ .../JsonSerializerInternalBase.cs | 33 ++ .../JsonSerializerInternalReader.cs | 36 +- .../JsonSerializerInternalWriter.cs | 20 +- src/Argon/Utilities/JavaScriptUtils.cs | 54 +-- src/ArgonTests/AssemblyInfo.cs | 2 +- .../Benchmarks/HotPathBenchmarks.cs | 318 ++++++++++++++++++ .../Converters/XmlNodeConverterTest.cs | 5 +- src/ArgonTests/JsonTextWriterTest.cs | 74 ++++ src/Benchmark.Tests/Program.cs | 9 +- todo.md | 173 ++++++++++ 19 files changed, 844 insertions(+), 61 deletions(-) create mode 100644 src/ArgonTests/Benchmarks/HotPathBenchmarks.cs create mode 100644 todo.md diff --git a/src/Argon.JsonPath/BooleanQueryExpression.cs b/src/Argon.JsonPath/BooleanQueryExpression.cs index e2fe6513c..32ff56168 100644 --- a/src/Argon.JsonPath/BooleanQueryExpression.cs +++ b/src/Argon.JsonPath/BooleanQueryExpression.cs @@ -17,6 +17,20 @@ class BooleanQueryExpression(QueryOperator @operator, object left, object? right readonly (string Pattern, RegexOptions Options)? regex = @operator == QueryOperator.RegexEquals ? ParseRegex(right) : null; + // the constructed Regex is cached with the timeout it was built for. evaluating via the + // static Regex.IsMatch costs a process-wide cache probe per candidate token, and silently + // degrades to a full pattern re-parse once more than Regex.CacheSize patterns are in play. + // built lazily so an invalid pattern still surfaces during evaluation rather than at parse + // time, and held as a single reference so a concurrently evaluated cached JPath can never + // observe a Regex paired with the wrong timeout + volatile CachedRegex? cachedRegex; + + sealed class CachedRegex(Regex regex, TimeSpan timeout) + { + public readonly Regex Regex = regex; + public readonly TimeSpan Timeout = timeout; + } + static (string Pattern, RegexOptions Options) ParseRegex(object? right) { if (right is not JValue {Value: string regexText}) @@ -194,9 +208,18 @@ bool RegexEquals(JValue input, JsonSelectSettings settings) return false; } - var (pattern, options) = regex!.Value; var timeout = settings.RegexMatchTimeout ?? Regex.InfiniteMatchTimeout; - return Regex.IsMatch((string) input.GetValue(), pattern, options, timeout); + + var cached = cachedRegex; + if (cached == null || + cached.Timeout != timeout) + { + var (pattern, options) = regex!.Value; + cached = new(new(pattern, options, timeout), timeout); + cachedRegex = cached; + } + + return cached.Regex.IsMatch((string) input.GetValue()); } static bool EqualsWithStringCoercion(JValue value, JValue queryValue) diff --git a/src/Argon/DefaultJsonNameTable.cs b/src/Argon/DefaultJsonNameTable.cs index 25b05da67..ed463f8b7 100644 --- a/src/Argon/DefaultJsonNameTable.cs +++ b/src/Argon/DefaultJsonNameTable.cs @@ -150,23 +150,9 @@ void Grow() Volatile.Write(ref mask, newMask); } - static bool TextEquals(string str1, char[] str2, int str2Start, int str2Length) - { - if (str1.Length != str2Length) - { - return false; - } - - for (var i = 0; i < str1.Length; i++) - { - if (str1[i] != str2[str2Start + i]) - { - return false; - } - } - - return true; - } + // SequenceEqual vectorizes the comparison and short circuits on a length mismatch + static bool TextEquals(string str1, char[] str2, int str2Start, int str2Length) => + str1.AsSpan().SequenceEqual(str2.AsSpan(str2Start, str2Length)); class Entry { diff --git a/src/Argon/JsonPosition.cs b/src/Argon/JsonPosition.cs index 30f873bbd..b151f4999 100644 --- a/src/Argon/JsonPosition.cs +++ b/src/Argon/JsonPosition.cs @@ -16,10 +16,29 @@ struct JsonPosition(JsonContainerType type) internal string? PropertyName = null; internal bool HasIndex = TypeHasIndex(type); + // set instead of PropertyName when the name arrived as a span. the chars are copied into a + // buffer owned by this position rather than materialized into a string, because the name is + // only ever read back to build a path for an error message + internal char[]? NameChars = null; + internal int NameLength = 0; + + readonly CharSpan Name + { + get + { + if (PropertyName != null) + { + return PropertyName.AsSpan(); + } + + return NameChars.AsSpan(0, NameLength); + } + } + int CalculateLength() => Type switch { - JsonContainerType.Object => PropertyName!.Length + 5, + JsonContainerType.Object => Name.Length + 5, JsonContainerType.Array => MathUtils.IntLength((ulong) Position) + 2, _ => throw new ArgumentOutOfRangeException(nameof(Type)) }; @@ -34,7 +53,7 @@ void WriteTo(StringBuilder builder, ref StringWriter? writer, ref char[]? buffer switch (Type) { case JsonContainerType.Object: - var propertyName = PropertyName!.AsSpan(); + var propertyName = Name; if (propertyName.IndexOfAny(specialCharacters) != -1) { builder.Append("['"); diff --git a/src/Argon/JsonTextReader.cs b/src/Argon/JsonTextReader.cs index fffdb66d6..8bf8fa052 100644 --- a/src/Argon/JsonTextReader.cs +++ b/src/Argon/JsonTextReader.cs @@ -963,6 +963,35 @@ bool ReadNullChar() return false; } +#if NET8_0_OR_GREATER + // every char a JSON string can contain that ReadStringIntoBuffer has to act on. '\0' is + // included because the buffer is always '\0' terminated at charsUsed, so the terminator + // doubles as the "need more data" sentinel and stops the scan at the end of valid content + static readonly SearchValues stringDelimiters = SearchValues.Create("\0\\\r\n\"'"); + + // vectorized skip past the run of chars needing no handling, so the per char switch in + // ReadStringIntoBuffer only runs on chars that actually do something. most strings contain + // none of these, so this collapses the whole scan into a single IndexOfAny + int SkipToNextStringDelimiter(int charPos) + { + var remaining = charsUsed - charPos; + if (remaining <= 0) + { + return charPos; + } + + var index = charBuffer.AsSpan(charPos, remaining).IndexOfAny(stringDelimiters); + if (index == -1) + { + // no delimiter in the buffered content, so jump to the '\0' terminator at + // charsUsed and let the switch trigger a read for more data + return charsUsed; + } + + return charPos + index; + } +#endif + void ReadStringIntoBuffer(char quote) { var charPos = this.charPos; @@ -972,6 +1001,9 @@ void ReadStringIntoBuffer(char quote) while (true) { +#if NET8_0_OR_GREATER + charPos = SkipToNextStringDelimiter(charPos); +#endif switch (charBuffer[charPos++]) { case '\0': diff --git a/src/Argon/JsonWriter.cs b/src/Argon/JsonWriter.cs index 5a9428d02..19fbe9e35 100644 --- a/src/Argon/JsonWriter.cs +++ b/src/Argon/JsonWriter.cs @@ -1517,8 +1517,25 @@ internal void InternalWritePropertyName(string name) AutoComplete(PropertyName); } - internal void InternalWritePropertyName(CharSpan name) => - InternalWritePropertyName(name.ToString()); + internal void InternalWritePropertyName(CharSpan name) + { + // copy the chars into a buffer owned by the current position instead of allocating a + // string. the buffer is reused by every property written at this depth, and the name is + // only read back when a path is built for an error message + var nameChars = currentPosition.NameChars; + if (nameChars == null || + nameChars.Length < name.Length) + { + nameChars = new char[Math.Max(name.Length, 16)]; + currentPosition.NameChars = nameChars; + } + + name.CopyTo(nameChars); + currentPosition.NameLength = name.Length; + currentPosition.PropertyName = null; + + AutoComplete(PropertyName); + } internal void InternalWriteStart(JsonToken token, JsonContainerType container) { diff --git a/src/Argon/Linq/JArray.cs b/src/Argon/Linq/JArray.cs index d6a296b97..edefc52aa 100644 --- a/src/Argon/Linq/JArray.cs +++ b/src/Argon/Linq/JArray.cs @@ -221,6 +221,11 @@ public JToken this[int index] set => SetItem(index, value); } + // index the backing list directly: the base implementation goes through the + // virtual ChildrenTokens property and then an IList interface dispatch + internal override JToken GetItem(int index) => + values[index]; + internal override int IndexOfItem(JToken? item) { if (item == null) diff --git a/src/Argon/Linq/JObject.cs b/src/Argon/Linq/JObject.cs index 9c36f4a20..8a44a5bc1 100644 --- a/src/Argon/Linq/JObject.cs +++ b/src/Argon/Linq/JObject.cs @@ -119,8 +119,15 @@ internal override JToken CloneToken() => /// Gets an of of this object's properties. /// /// An of of this object's properties. - public IEnumerable Properties() => - properties.Cast(); + public IEnumerable Properties() + { + // iterate InnerList rather than using Cast: avoids the LINQ wrapper + // enumerable and the boxed interface enumerator on every call + foreach (var token in properties.InnerList) + { + yield return (JProperty) token; + } + } /// /// Gets the with the specified name. @@ -502,8 +509,9 @@ public bool TryGetValue(string propertyName, [NotNullWhen(true)] out JToken? val } var index = 0; - foreach (JProperty property in properties) + foreach (var token in properties.InnerList) { + var property = (JProperty) token; array[arrayIndex + index] = new(property.Name, property.Value); index++; } diff --git a/src/Argon/Linq/JTokenWriter.cs b/src/Argon/Linq/JTokenWriter.cs index 9d5509837..9d851e710 100644 --- a/src/Argon/Linq/JTokenWriter.cs +++ b/src/Argon/Linq/JTokenWriter.cs @@ -283,7 +283,9 @@ public override void WriteValue(CharSpan value) public override void WriteValue(int value) { base.WriteValue(value); - AddRawValue(value, JTokenType.Integer); + // int is the most common CLR type for a JSON number, so use the shared + // boxes for small values instead of boxing at the call site + AddRawValue(BoxedPrimitives.Get(value), JTokenType.Integer); } /// diff --git a/src/Argon/Serialization/JsonObjectContract.cs b/src/Argon/Serialization/JsonObjectContract.cs index 1f4167d0f..1214e172a 100644 --- a/src/Argon/Serialization/JsonObjectContract.cs +++ b/src/Argon/Serialization/JsonObjectContract.cs @@ -48,6 +48,41 @@ public class JsonObjectContract : JsonContainerContract internal ObjectConstructor? ParameterizedCreator { get; set; } + Dictionary? creatorParameterIndexes; + + /// + /// Gets the index of a creator parameter within , or -1 when it is not one. + /// + /// + /// Backed by a lookup rather than a scan of : the index is needed + /// once per matched parameter while deserializing, so scanning makes every object built through a + /// parameterized constructor - records and immutable types - quadratic in its parameter count. + /// Built on first use because the contract resolver populates + /// after the contract is constructed, and rebuilt if that collection is later changed. + /// + internal int IndexOfCreatorParameter(JsonProperty property) + { + var indexes = creatorParameterIndexes; + if (indexes == null || + indexes.Count != CreatorParameters.Count) + { + indexes = new(CreatorParameters.Count); + for (var index = 0; index < CreatorParameters.Count; index++) + { + indexes[CreatorParameters[index]] = index; + } + + creatorParameterIndexes = indexes; + } + + if (indexes.TryGetValue(property, out var result)) + { + return result; + } + + return -1; + } + bool? hasRequiredOrDefaultValueProperties; internal bool HasRequiredOrDefaultValueProperties diff --git a/src/Argon/Serialization/JsonSerializerInternalBase.cs b/src/Argon/Serialization/JsonSerializerInternalBase.cs index 03ed3b086..7eb13ffb0 100644 --- a/src/Argon/Serialization/JsonSerializerInternalBase.cs +++ b/src/Argon/Serialization/JsonSerializerInternalBase.cs @@ -40,6 +40,39 @@ protected static bool HasFlag(DefaultValueHandling? value, DefaultValueHandling "A different value already has the Id '{0}'.", "A different Id has already been assigned for value '{0}'. This error may be caused by an object being reused multiple times during deserialization and can be fixed with the setting ObjectCreationHandling.Replace."); + Dictionary? matchingConverters; + + /// + /// Resolves the converter for a type from the converters registered on the serializer. + /// + /// + /// Memoizes the scan. The underlying lookup calls virtual + /// on every registered converter, and it runs for every value serialized and every property, + /// collection item and dictionary entry deserialized. The result cannot be cached on the + /// contract because contracts are shared between serializers with different converter lists, + /// so it is cached here instead - a fresh instance is created for each serialize/deserialize + /// call, so converters registered between calls are always picked up. + /// + protected JsonConverter? GetMatchingConverter(Type type) + { + var converters = Serializer.Converters; + if (converters.Count == 0) + { + return null; + } + + matchingConverters ??= []; + + if (matchingConverters.TryGetValue(type, out var converter)) + { + return converter; + } + + converter = Serializer.GetMatchingConverter(type); + matchingConverters[type] = converter; + return converter; + } + protected NullValueHandling ResolvedNullValueHandling(JsonObjectContract? containerContract, JsonProperty property) => property.NullValueHandling ?? containerContract?.ItemNullValueHandling ?? diff --git a/src/Argon/Serialization/JsonSerializerInternalReader.cs b/src/Argon/Serialization/JsonSerializerInternalReader.cs index 9828c6100..0254df505 100644 --- a/src/Argon/Serialization/JsonSerializerInternalReader.cs +++ b/src/Argon/Serialization/JsonSerializerInternalReader.cs @@ -3,6 +3,11 @@ // as found in the license.md file. +#if NET6_0_OR_GREATER +using System.Runtime.CompilerServices; +using System.Runtime.InteropServices; +#endif + // ReSharper disable NullableWarningSuppressionIsUsed // ReSharper disable RedundantSuppressNullableWarningExpression @@ -273,7 +278,7 @@ static string GetExpectedDescription(JsonContract contract) return contract.Converter; } - if (Serializer.GetMatchingConverter(contract.UnderlyingType) is { } matchingConverter) + if (GetMatchingConverter(contract.UnderlyingType) is { } matchingConverter) { // passed in converters return matchingConverter; @@ -978,8 +983,9 @@ JsonToken.StartArray or } else { - propertyContract = GetContract(currentValue.GetType()); - + // currentValue is only ever non null when the block above ran, and that already + // resolved propertyContract from the same currentValue.GetType(). resolving it + // again would repeat a GetType and a contract dictionary lookup per property if (propertyContract != property.PropertyContract) { propertyConverter = GetConverter(propertyContract, property.Converter, containerContract, containerProperty); @@ -1746,7 +1752,7 @@ object CreateObjectUsingCreatorWithParameters(JsonReader reader, JsonObjectContr } } - var i = contract.CreatorParameters.IndexOf(constructorProperty); + var i = contract.IndexOfCreatorParameter(constructorProperty); creatorParameterValues[i] = context.Value; context.Used = true; @@ -2228,11 +2234,25 @@ Required.AllowNull or static void SetPropertyPresence(JsonReader reader, JsonProperty property, Dictionary? requiredProperties) { // the dictionary only tracks presence-relevant properties; do not add others back - if (requiredProperties == null || - !requiredProperties.ContainsKey(property)) + if (requiredProperties == null) + { + return; + } + +#if NET6_0_OR_GREATER + // one hash lookup rather than a ContainsKey probe followed by an indexer set, + // which runs for every property of every object with tracked presence + ref var presenceSlot = ref CollectionsMarshal.GetValueRefOrNullRef(requiredProperties, property); + if (Unsafe.IsNullRef(ref presenceSlot)) + { + return; + } +#else + if (!requiredProperties.ContainsKey(property)) { return; } +#endif PropertyPresence propertyPresence; switch (reader.TokenType) @@ -2257,7 +2277,11 @@ static void SetPropertyPresence(JsonReader reader, JsonProperty property, Dictio break; } +#if NET6_0_OR_GREATER + presenceSlot = propertyPresence; +#else requiredProperties[property] = propertyPresence; +#endif } void HandleError(JsonReader reader, bool readPastError, int initialDepth) diff --git a/src/Argon/Serialization/JsonSerializerInternalWriter.cs b/src/Argon/Serialization/JsonSerializerInternalWriter.cs index d719b0719..057ec787a 100644 --- a/src/Argon/Serialization/JsonSerializerInternalWriter.cs +++ b/src/Argon/Serialization/JsonSerializerInternalWriter.cs @@ -14,6 +14,13 @@ class JsonSerializerInternalWriter(JsonSerializer serializer) : int rootLevel; readonly List serializeStack = []; + // formatted $type names, cached for the duration of this serialization. building one + // concatenates the type and assembly names and then re-parses the result through + // RemoveAssemblyDetails, allocating a StringBuilder and a string, and polymorphic payloads + // repeat the same handful of types over and over. the binder and format handling are fixed + // for a run, so the type alone is a sufficient key + Dictionary? typeNames; + [RequiresUnreferencedCode(MiscellaneousUtils.TrimWarning)] [RequiresDynamicCode(MiscellaneousUtils.AotWarning)] public void Serialize(JsonWriter jsonWriter, object? value, Type? type) @@ -116,7 +123,7 @@ void SerializeValue(JsonWriter writer, object? value, JsonContract? valueContrac containerProperty?.ItemConverter ?? containerContract?.ItemConverter ?? valueContract.Converter ?? - Serializer.GetMatchingConverter(valueContract.UnderlyingType) ?? + GetMatchingConverter(valueContract.UnderlyingType) ?? valueContract.InternalConverter; if (converter is {CanWrite: true}) @@ -507,8 +514,15 @@ void WriteReferenceIdProperty(JsonWriter writer, object value) void WriteTypeProperty(JsonWriter writer, Type type) { - var binder = Serializer.SerializationBinder ?? DefaultSerializationBinder.Instance; - var typeName = type.GetTypeName(Serializer.TypeNameAssemblyFormatHandling, binder); + typeNames ??= []; + + if (!typeNames.TryGetValue(type, out var typeName)) + { + var binder = Serializer.SerializationBinder ?? DefaultSerializationBinder.Instance; + typeName = type.GetTypeName(Serializer.TypeNameAssemblyFormatHandling, binder); + typeNames[type] = typeName; + } + writer.WritePropertyName(JsonTypeReflector.TypePropertyName, false); writer.WriteValue(typeName); } diff --git a/src/Argon/Utilities/JavaScriptUtils.cs b/src/Argon/Utilities/JavaScriptUtils.cs index 2a564ecdb..1d9b2302b 100644 --- a/src/Argon/Utilities/JavaScriptUtils.cs +++ b/src/Argon/Utilities/JavaScriptUtils.cs @@ -170,15 +170,11 @@ static void WriteEscapedJavaScriptNonNullString(TextWriter writer, CharSpan valu writer.Write(value.Slice(0, lastWritePosition)); } - for (var i = lastWritePosition; i < value.Length; i++) + var i = lastWritePosition; + while (true) { var c = value[i]; - if (c < escapeFlags.Length && !escapeFlags[c]) - { - continue; - } - string? escapedValue; switch (c) @@ -245,31 +241,45 @@ static void WriteEscapedJavaScriptNonNullString(TextWriter writer, CharSpan valu break; } - if (escapedValue == null) + if (escapedValue != null) { - continue; - } + // Safe to use ReferenceEquals: escapedValue is either null (handled above), + // a string literal from the switch branches, or the escapedUnicodeText sentinel + // assigned directly in the default branch. No other branch produces "!". + var isEscapedUnicodeText = ReferenceEquals(escapedValue, escapedUnicodeText); - // Safe to use ReferenceEquals: escapedValue is either null (handled above), - // a string literal from the switch branches, or the escapedUnicodeText sentinel - // assigned directly at line 199. No other branch produces a string equal to "!". - var isEscapedUnicodeText = ReferenceEquals(escapedValue, escapedUnicodeText); + if (i > lastWritePosition) + { + // write unchanged chars before writing escaped text + writer.Write(value.Slice(lastWritePosition, i - lastWritePosition)); + } - if (i > lastWritePosition) - { - // write unchanged chars before writing escaped text - writer.Write(value.Slice(lastWritePosition, i - lastWritePosition)); + lastWritePosition = i + 1; + if (isEscapedUnicodeText) + { + writer.Write(buffer!, 0, unicodeTextLength); + } + else + { + writer.Write(escapedValue); + } } - lastWritePosition = i + 1; - if (isEscapedUnicodeText) + // jump to the next char needing an escape rather than stepping over the + // clean run one char at a time; FirstCharToEscape vectorizes the scan + i++; + if (i == value.Length) { - writer.Write(buffer!, 0, unicodeTextLength); + break; } - else + + var next = FirstCharToEscape(value.Slice(i), escapeFlags, escapeHandling); + if (next == -1) { - writer.Write(escapedValue); + break; } + + i += next; } MiscellaneousUtils.Assert(lastWritePosition != 0); diff --git a/src/ArgonTests/AssemblyInfo.cs b/src/ArgonTests/AssemblyInfo.cs index f4ae2a59d..d10339d7d 100644 --- a/src/ArgonTests/AssemblyInfo.cs +++ b/src/ArgonTests/AssemblyInfo.cs @@ -2,4 +2,4 @@ // Use of this source code is governed by The MIT License, // as found in the license.md file. -[assembly: CollectionBehavior(DisableTestParallelization = true)] \ No newline at end of file +[assembly: Xunit.v3.Parallelization(Mode = Xunit.Sdk.ParallelMode.None)] \ No newline at end of file diff --git a/src/ArgonTests/Benchmarks/HotPathBenchmarks.cs b/src/ArgonTests/Benchmarks/HotPathBenchmarks.cs new file mode 100644 index 000000000..4eba283ba --- /dev/null +++ b/src/ArgonTests/Benchmarks/HotPathBenchmarks.cs @@ -0,0 +1,318 @@ +// Copyright (c) 2007 James Newton-King. All rights reserved. +// Use of this source code is governed by The MIT License, +// as found in the license.md file. + +using BenchmarkDotNet.Attributes; + +// Benchmarks covering the hot path fixes from the performance review in todo.md. +// +// The reader string scan and the escape writer fixes are already measured by +// ReaderBenchmarks.ReadStringHeavy and WriterBenchmarks.WriteEscapedStrings/WriteCleanStrings, +// and the JsonPath regex fix by JsonPathRegexBenchmark, so they are not repeated here. + +// JsonWriter.InternalWritePropertyName(CharSpan): the span overload used to call ToString on the +// span so the name could be stored for path tracking, allocating exactly as much as the string +// overload did and defeating the point of the API. The chars are now copied into a buffer that is +// reused by every property written at that depth, so a whole object costs one buffer rather than +// one string per property. +[MemoryDiagnoser] +public class SpanPropertyNameBenchmark +{ + string source; + (int Start, int Length)[] slices; + + [GlobalSetup] + public void Setup() + { + // names sliced out of a larger buffer, which is what the span overload exists for + var builder = new StringBuilder(); + var found = new List<(int, int)>(); + foreach (var index in Enumerable.Range(0, 20)) + { + var name = $"someProperty{index}"; + found.Add((builder.Length, name.Length)); + builder.Append(name); + } + + source = builder.ToString(); + slices = found.ToArray(); + } + + [Benchmark] + public string WriteSpanPropertyNames() + { + var stringWriter = new StringWriter(); + using (var writer = new JsonTextWriter(stringWriter)) + { + writer.WriteStartObject(); + foreach (var slice in slices) + { + writer.WritePropertyName(source.AsSpan(slice.Start, slice.Length)); + writer.WriteValue(1); + } + + writer.WriteEndObject(); + } + + return stringWriter.ToString(); + } +} + +// DefaultJsonNameTable.Get: the interned name comparison behind every property name lookup during +// deserialization was a per character loop, now a vectorized SequenceEqual. +[MemoryDiagnoser] +public class NameTableGetBenchmark +{ + DefaultJsonNameTable table; + char[] buffer; + (int Start, int Length)[] lookups; + + [GlobalSetup] + public void Setup() + { + table = new(); + var builder = new StringBuilder(); + var found = new List<(int, int)>(); + foreach (var index in Enumerable.Range(0, 64)) + { + // long enough for the comparison to be worth vectorizing, which is the realistic + // shape for the camel/pascal case property names of a typical model + var name = $"aFairlyLongPropertyName{index}"; + table.Add(name); + found.Add((builder.Length, name.Length)); + builder.Append(name); + } + + buffer = builder.ToString().ToCharArray(); + lookups = found.ToArray(); + } + + [Benchmark] + public string GetInternedNames() + { + string last = null; + foreach (var lookup in lookups) + { + last = table.Get(buffer, lookup.Start, lookup.Length); + } + + return last; + } +} + +// JsonSerializerInternalBase.GetMatchingConverter: the registered converter list was rescanned, +// calling virtual CanConvert on each converter, for every value written and every property, +// collection item and dictionary entry read. Resolution is now memoized for the duration of a +// single serialize or deserialize call. +[MemoryDiagnoser] +public class ConverterLookupBenchmark +{ + JsonSerializerSettings settings; + ConverterModel[] models; + string json; + + [GlobalSetup] + public void Setup() + { + settings = new() + { + Converters = + { + new VersionConverter(), + new StringEnumConverter(), + new IsoDateTimeConverter(), + new EncodingConverter(), + new StringBuilderConverter(), + new TimeZoneInfoConverter() + } + }; + + models = Enumerable.Range(0, 100) + .Select(_ => new ConverterModel + { + Name = $"item{_}", + Value = _, + Ratio = _ * 1.5, + Flag = _ % 2 == 0 + }) + .ToArray(); + + json = JsonConvert.SerializeObject(models, settings); + } + + [Benchmark] + public string SerializeWithConverters() => + JsonConvert.SerializeObject(models, settings); + + [Benchmark] + public ConverterModel[] DeserializeWithConverters() => + JsonConvert.DeserializeObject(json, settings); + + public class ConverterModel + { + public string Name { get; set; } + public int Value { get; set; } + public double Ratio { get; set; } + public bool Flag { get; set; } + } +} + +// JsonSerializerInternalWriter.WriteTypeProperty: the $type string was rebuilt for every object +// written, concatenating the type and assembly names and then re-parsing the result through +// RemoveAssemblyDetails. Polymorphic payloads repeat the same few types, so it is now cached for +// the duration of the serialization. +[MemoryDiagnoser] +public class TypeNameWriteBenchmark +{ + object[] items; + JsonSerializerSettings settings; + + [GlobalSetup] + public void Setup() + { + settings = new() + { + TypeNameHandling = TypeNameHandling.All + }; + + items = Enumerable.Range(0, 200) + .Select(object (_) => new TypeNameItem + { + Name = $"item{_}", + Value = _ + }) + .ToArray(); + } + + [Benchmark] + public string SerializeWithTypeNames() => + JsonConvert.SerializeObject(items, settings); + + public class TypeNameItem + { + public string Name { get; set; } + public int Value { get; set; } + } +} + +// JsonObjectContract.IndexOfCreatorParameter: locating the argument slot for a matched +// constructor parameter was an IndexOf scan of CreatorParameters, so every object built through a +// parameterized constructor was quadratic in its parameter count. Records and other immutable +// types with a wide constructor paid the most, so this uses a deliberately wide one. +[MemoryDiagnoser] +public class WideCreatorBenchmark +{ + string json; + + [GlobalSetup] + public void Setup() => + json = JsonConvert.SerializeObject( + new WideRecord(1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16)); + + [Benchmark] + public WideRecord DeserializeWideRecord() => + JsonConvert.DeserializeObject(json); + + public record WideRecord( + int A1, int A2, int A3, int A4, + int A5, int A6, int A7, int A8, + int A9, int A10, int A11, int A12, + int A13, int A14, int A15, int A16); +} + +// JsonSerializerInternalReader.CalculatePropertyDetails: the contract for an existing property +// value was resolved twice, so populating an object paid two GetType calls and two contract +// lookups for every read only property that gets populated in place. +[MemoryDiagnoser] +public class PopulateExistingBenchmark +{ + string json; + + [GlobalSetup] + public void Setup() => + json = JsonConvert.SerializeObject( + new PopulateTarget + { + Numbers = {1, 2, 3, 4, 5}, + Names = {"one", "two", "three"}, + Map = {["a"] = 1, ["b"] = 2}, + Nested = {Numbers = {9, 8, 7}} + }); + + [Benchmark] + public PopulateTarget PopulateExistingValues() => + JsonConvert.DeserializeObject(json); + + public class PopulateTarget + { + // read only and initialized by the constructor, so the serializer populates the existing + // instance in place rather than replacing it, which is the path that resolved the + // existing value's contract twice + public List Numbers { get; } = []; + public List Names { get; } = []; + public Dictionary Map { get; } = []; + public NestedTarget Nested { get; } = new(); + } + + public class NestedTarget + { + public List Numbers { get; } = []; + } +} + +// Linq to JSON hot paths: +// - JObject.Properties() built a LINQ Cast wrapper plus a boxed enumerator per call +// - JTokenWriter.WriteValue(int) boxed at the call site while the other numeric overloads +// already used the shared BoxedPrimitives boxes +// - JArray's indexer reached the backing list through a virtual property and an interface +// dispatch rather than indexing it directly +[MemoryDiagnoser] +public class JTokenHotPathBenchmark +{ + JObject document; + int[] numbers; + JArray array; + + [GlobalSetup] + public void Setup() + { + document = new( + Enumerable.Range(0, 50) + .Select(_ => new JProperty($"property{_}", _))); + + // weighted to the small values BoxedPrimitives caches, as JSON payloads tend to be + numbers = Enumerable.Range(0, 500) + .Select(_ => _ % 9) + .ToArray(); + + array = new(Enumerable.Range(0, 500)); + } + + [Benchmark] + public int EnumerateProperties() + { + var total = 0; + foreach (var property in document.Properties()) + { + total += property.Name.Length; + } + + return total; + } + + [Benchmark] + public JToken IntArrayFromObject() => + JToken.FromObject(numbers); + + [Benchmark] + public int IndexArray() + { + var total = 0; + for (var index = 0; index < array.Count; index++) + { + total += (int) array[index]; + } + + return total; + } +} diff --git a/src/ArgonTests/Converters/XmlNodeConverterTest.cs b/src/ArgonTests/Converters/XmlNodeConverterTest.cs index ed54302e8..b223d7528 100644 --- a/src/ArgonTests/Converters/XmlNodeConverterTest.cs +++ b/src/ArgonTests/Converters/XmlNodeConverterTest.cs @@ -727,7 +727,10 @@ public class DecimalContainer [Fact] public void FloatParseHandlingDecimal() { - var d = (decimal) Math.PI + 1000000000m; + // a decimal literal rather than (decimal) Math.PI: the double to decimal conversion + // returns full precision on modern runtimes, so casting would make this a test of + // conversion precision rather than of the XML/JSON round trip + var d = 1000000003.14159265358979m; var x = new DecimalContainer {Number = d}; var json = JsonConvert.SerializeObject(x, Formatting.Indented); diff --git a/src/ArgonTests/JsonTextWriterTest.cs b/src/ArgonTests/JsonTextWriterTest.cs index a52b7bec3..20d1450f1 100644 --- a/src/ArgonTests/JsonTextWriterTest.cs +++ b/src/ArgonTests/JsonTextWriterTest.cs @@ -1210,6 +1210,80 @@ public Task Path() return Verify(stringWriter); } + [Fact] + public void PathWithSpanPropertyNames() + { + // the span overload does not materialize the name into a string, it copies the chars into + // a buffer reused across the properties written at that depth, so the path has to stay + // correct as names change length, as depths are pushed and popped, and when the two + // overloads are mixed + var stringWriter = new StringWriter(); + + using (var jsonWriter = new JsonTextWriter(stringWriter)) + { + jsonWriter.WriteStartArray(); + jsonWriter.WriteStartObject(); + + jsonWriter.WritePropertyName("Property1".AsSpan()); + Assert.Equal("[0].Property1", jsonWriter.Path); + jsonWriter.WriteValue(1); + + // shorter name over a buffer a longer one already wrote into + jsonWriter.WritePropertyName("P2".AsSpan()); + Assert.Equal("[0].P2", jsonWriter.Path); + + // the parent name has to survive being pushed and popped + jsonWriter.WriteStartObject(); + jsonWriter.WritePropertyName("Child".AsSpan()); + Assert.Equal("[0].P2.Child", jsonWriter.Path); + jsonWriter.WriteValue(2); + jsonWriter.WriteEndObject(); + + jsonWriter.WritePropertyName("AfterNested".AsSpan()); + Assert.Equal("[0].AfterNested", jsonWriter.Path); + jsonWriter.WriteValue(3); + + // names with special characters take the escaping branch of the path builder + jsonWriter.WritePropertyName("has space".AsSpan()); + Assert.Equal("[0]['has space']", jsonWriter.Path); + jsonWriter.WriteValue(4); + + // a slice of a larger buffer must not pick up the chars around it + jsonWriter.WritePropertyName("XXXSlicedXXX".AsSpan(3, 6)); + Assert.Equal("[0].Sliced", jsonWriter.Path); + jsonWriter.WriteValue(5); + + // back to the string overload at the same depth + jsonWriter.WritePropertyName("Plain"); + Assert.Equal("[0].Plain", jsonWriter.Path); + jsonWriter.WriteValue(6); + + jsonWriter.WriteEndObject(); + jsonWriter.WriteEndArray(); + } + + Assert.Equal( + """[{"Property1":1,"P2":{"Child":2},"AfterNested":3,"has space":4,"Sliced":5,"Plain":6}]""", + stringWriter.ToString()); + } + + [Fact] + public void SpanPropertyNameInExceptionPath() + { + // the deferred name is read back when an error path is built, which is the only thing + // that consumes it + var stringWriter = new StringWriter(); + var jsonWriter = new JsonTextWriter(stringWriter); + jsonWriter.WriteStartObject(); + jsonWriter.WritePropertyName("badProperty".AsSpan()); + // nest so the position holding the span name is on the stack that ContainerPath builds from + jsonWriter.WriteStartObject(); + + var exception = Assert.Throws(() => jsonWriter.WriteValue(new Version(1, 2))); + + Assert.Contains("Path 'badProperty'", exception.Message); + } + [Fact] public Task BuildStateArray() { diff --git a/src/Benchmark.Tests/Program.cs b/src/Benchmark.Tests/Program.cs index 49f1fb93b..5286cc242 100644 --- a/src/Benchmark.Tests/Program.cs +++ b/src/Benchmark.Tests/Program.cs @@ -30,7 +30,14 @@ public static void Main(string[] args) typeof(NameTableAddBenchmark), typeof(EscapeToStringBenchmark), typeof(EnumWriteBenchmark), - typeof(JsonPathRegexBenchmark) + typeof(JsonPathRegexBenchmark), + typeof(SpanPropertyNameBenchmark), + typeof(NameTableGetBenchmark), + typeof(ConverterLookupBenchmark), + typeof(TypeNameWriteBenchmark), + typeof(WideCreatorBenchmark), + typeof(PopulateExistingBenchmark), + typeof(JTokenHotPathBenchmark) ]); if (args.Length == 0) { diff --git a/todo.md b/todo.md new file mode 100644 index 000000000..7d7d7231e --- /dev/null +++ b/todo.md @@ -0,0 +1,173 @@ +# Performance improvement todo + +Findings from a perf review of the core read/write path, serialization, and LINQ-to-JSON / JSONPath (2026-08-27). +Prioritized within each section; high-priority items sit on per-character / per-token / per-property hot paths. + +**All high-priority items are implemented.** Medium and low priority items remain open. + +## Measured impact + +BenchmarkDotNet `--job short`, .NET 10.0.11, AMD Ryzen 9 5900X. Baseline is a clean worktree at the +commit these changes sit on, running the identical benchmark code. Benchmarks live in +[HotPathBenchmarks.cs](src/ArgonTests/Benchmarks/HotPathBenchmarks.cs), plus the pre-existing +`ReaderBenchmarks`, `WriterBenchmarks` and `JsonPathRegexBenchmark`. + +| Benchmark | Before | After | Time | Allocation | +|---|---|---|---|---| +| `ReadStringHeavy` | 5.727 µs | 1.611 µs | **3.6× faster** | unchanged | +| `SerializeWithTypeNames` | 121.2 µs / 265 KB | 36.21 µs / 89.9 KB | **3.3× faster** | **−66%** | +| `SerializeWithConverters` | 70.65 µs | 32.56 µs | **2.2× faster** | +0.5 KB (memo) | +| `GetInternedNames` | 2.081 µs | 1.083 µs | **1.9× faster** | none either way | +| `DeserializeWithConverters` | 86.20 µs | 47.20 µs | **1.8× faster** | +0.5 KB (memo) | +| `WriteSpanPropertyNames` | 1.208 µs / 3.46 KB | 693.5 ns / 2.51 KB | **1.7× faster** | **−27%** | +| `DeserializeWideRecord` | 2.544 µs | 1.628 µs | **1.6× faster** | unchanged | +| `EnumerateProperties` | 301.8 ns / 88 B | 227.2 ns / 64 B | **1.3× faster** | **−27%** | +| `PopulateExistingValues` | 1.727 µs | 1.317 µs | **1.3× faster** | +0.05 KB | +| `IndexArray` | 2.107 µs | 1.998 µs | 1.05× faster | none either way | +| `IntArrayFromObject` | 27.5 µs / 69.1 KB | 29.6 µs / 57.1 KB | within noise | **−17%** | + +Notes on the two rows that are not a clean win: + +- The converter memo adds ~0.5 KB per serialize/deserialize call for the `Dictionary` + itself. That buys roughly a halving of wall-clock whenever converters are registered, and the + dictionary is never allocated at all when the converter list is empty. +- `IntArrayFromObject` time moved within the measurement error of both runs (the confidence intervals + overlap); the 17% allocation drop from routing `WriteValue(int)` through `BoxedPrimitives` is the real + and repeatable part. + +## Core reader / writer + +### High priority — done + +- [x] **Vectorize `ReadStringIntoBuffer`** — [JsonTextReader.cs](src/Argon/JsonTextReader.cs). + The hottest loop in the reader scanned every string char-by-char through a scalar `switch`, but typical strings contain none of the six interesting chars (`\0 \\ \r \n " '`). Added `SkipToNextStringDelimiter`, which uses a static `SearchValues` to jump straight to the next char the switch actually acts on; the `'\0'` terminator kept at `charsUsed` doubles as the scan's stop sentinel, so the "need more data" path is unchanged. Guarded to net8+, with the original scalar walk on older TFMs. **3.6× faster** on `ReadStringHeavy`. + +- [x] **Stop allocating a string per property in span-based `WritePropertyName`** — [JsonWriter.cs](src/Argon/JsonWriter.cs), [JsonPosition.cs](src/Argon/JsonPosition.cs). + `InternalWritePropertyName(name.ToString())` materialized the span on every call, so the span overload allocated exactly as much as the string one. `JsonPosition` now also holds `NameChars`/`NameLength`, and the span overload copies into a buffer owned by the position and reused by every property at that depth. Path building reads whichever of the two is set. **1.7× faster, 27% less allocation.** + +- [x] **Vectorize `DefaultJsonNameTable.TextEquals`** — [DefaultJsonNameTable.cs](src/Argon/DefaultJsonNameTable.cs). + Replaced the manual char loop with `str1.AsSpan().SequenceEqual(str2.AsSpan(str2Start, str2Length))`, which also short circuits the length mismatch. **1.9× faster** on `GetInternedNames`. + +- [x] **Keep the escape writer vectorized after the first escape** — [JavaScriptUtils.cs](src/Argon/Utilities/JavaScriptUtils.cs). + `WriteEscapedJavaScriptNonNullString` used the vectorized `FirstCharToEscape` only to find the first escapable char and then walked the rest of the string one char at a time. It now re-runs that scan on the remaining slice after each escape, so the clean runs between escapes are skipped rather than stepped over. + +### Medium priority (per-value allocations) + +- [ ] **Span-based Guid probe in `ReadAsBytes`** — `src/Argon/JsonTextReader.cs:97-98`. + `TryConvertGuid(stringReference.ToString(), ...)` allocates a 36-char string for every 36-char byte-string, even when it's base64. Add a `TryConvertGuid(CharSpan)` overload in `ConvertUtils` (`Guid.TryParseExact` has span overloads via Polyfill) and pass `stringReference.AsSpan()`. + +- [ ] **Stackalloc DateTime write buffers** — `src/Argon/Utilities/DateTimeUtils.cs:136-141, 224-230`. + `WriteDateTimeString` / `WriteDateTimeOffsetString` allocate `new char[64]` per call (hot for date-keyed dictionaries via `JsonSerializerInternalWriter.cs:1051`). Use `stackalloc char[64]` and span-based helpers. + +- [ ] **Fast-path `ToEscapedJavaScriptString` when nothing needs escaping** — `src/Argon/Utilities/JavaScriptUtils.cs:283-300`. + Always builds via `StringWriter` → `StringBuilder` → `ToString()`. When `FirstCharToEscape` returns -1, build the result directly (`string.Create` on modern TFMs, or `string.Concat` with the delimiters). + +- [ ] **Span parameters for `DecimalTryParse` / `Int32TryParse` / `Int64TryParse`** — `src/Argon/JsonReader.cs:760, 786`, `src/Argon/Utilities/ConvertUtils.cs:551, 645, 737`. + `ReadDecimalString` pays `s.ToCharArray()` on every exponent-form decimal (`"96.014e-05"`), a legitimate parse path. The parsers only index their input — change `char[] chars, int start, int length` to `ReadOnlySpan` and pass spans everywhere; no copies. + +- [ ] **`JsonConvert.ToString(char)` allocates a temp array** — `src/Argon/JsonConvert.cs:92-93`. + `new[]{value}.AsSpan()` heap-allocates per call; use `stackalloc char[1]` or `new ReadOnlySpan(in value)`. + +### Low priority + +- [ ] **Integer math in `ShiftBufferIfNeeded`** — `src/Argon/JsonTextReader.cs:141`. + `length - charPos <= length * 0.1` does double conversion/multiply once per string/number token; use `(length - charPos) * 10L <= length`. + +- [ ] **(Benchmark first) `ReadNumberIntoBuffer` per-char switch** — `src/Argon/JsonTextReader.cs:1172-1248`. + 28-case switch per digit; `IndexOfAnyExcept` with `SearchValues` of `[0-9a-fA-FxX.+-]` would find the terminator in one call, but numbers are usually short — measure before doing. + +## Serialization + +### High priority — done + +- [x] **Memoize the per-value converter scan** — [JsonSerializerInternalBase.cs](src/Argon/Serialization/JsonSerializerInternalBase.cs), used from [JsonSerializerInternalWriter.cs](src/Argon/Serialization/JsonSerializerInternalWriter.cs) and [JsonSerializerInternalReader.cs](src/Argon/Serialization/JsonSerializerInternalReader.cs). + `GetMatchingConverter` walked the converter list calling virtual `CanConvert(type)` for every value serialized and every property, item and dictionary entry deserialized. Now memoized in a `Dictionary` on the internal base, which is instantiated fresh per serialize/deserialize call, so converters registered between calls are still picked up. Not cacheable on the contract, since contracts are shared across serializers with different converter lists. **2.2× faster serializing, 1.8× deserializing** with converters registered. + +- [x] **Cache formatted `$type` names** — [JsonSerializerInternalWriter.cs](src/Argon/Serialization/JsonSerializerInternalWriter.cs). + Every `$type` concatenated the type and assembly names and re-parsed the result through `RemoveAssemblyDetails`, allocating a `StringBuilder` and a string per object. Now cached per run, keyed on type alone — the binder and format handling are fixed for the duration of a serialization. **3.3× faster, 66% less allocation.** + +- [x] **Drop duplicate contract resolution in `CalculatePropertyDetails`** — [JsonSerializerInternalReader.cs](src/Argon/Serialization/JsonSerializerInternalReader.cs). + `GetContract(currentValue.GetType())` ran twice with the same argument per populated property. `currentValue` is only ever non-null when the earlier block ran, and that block already resolved the same contract, so the second resolution is gone. **1.3× faster** on `PopulateExistingValues`. + +- [x] **Fix O(n²) creator-parameter index lookup** — [JsonObjectContract.cs](src/Argon/Serialization/JsonObjectContract.cs). + `CreatorParameters.IndexOf(constructorProperty)` was a linear scan per matched parameter, making every object built through a parameterized constructor quadratic in its parameter count. Added `IndexOfCreatorParameter`, backed by a `Dictionary` built on first use (the resolver populates `CreatorParameters` after construction) and rebuilt if that collection later changes. **1.6× faster** deserializing a 16-parameter record. + +- [x] **Single-lookup `SetPropertyPresence`** — [JsonSerializerInternalReader.cs](src/Argon/Serialization/JsonSerializerInternalReader.cs). + `ContainsKey` followed by an indexer set hashed the key twice per property. Uses `CollectionsMarshal.GetValueRefOrNullRef` on net6+, with the original two-lookup form kept under `#if` for net4x. + +### Medium priority + +- [ ] **Cache transformed dictionary keys in naming strategies** — `src/Argon/NamingStrategy/NamingStrategy.cs:46-54` (+ snake/kebab/camel implementations), reached via `DefaultContractResolver.cs:905-913` from `JsonSerializerInternalWriter.cs:991-994`. + With `ProcessDictionaryKeys = true`, every dictionary entry pays a case-conversion allocation per serialization call for keys that repeat across calls. Add a bounded `ThreadSafeStore` cache (key space is user data — cap growth). + +- [ ] **Kill the StringWriter per DateTime dictionary key** — `src/Argon/Serialization/JsonSerializerInternalWriter.cs:1045-1059`. + `GetDictionaryPropertyName` allocates a `StringWriter` + `StringBuilder` per date key; add direct string-returning overloads in `DateTimeUtils` (pairs with the stackalloc item above). + +- [ ] **Cache `$type` name splitting during deserialization** — `src/Argon/Serialization/JsonSerializerInternalReader.cs:646-654`. + `SplitFullyQualifiedTypeName` allocates substrings per `$type` occurrence; only `BindToType` is cached. Cache `string -> TypeNameKey` (bounded — input is untrusted JSON). + +### Low priority + +- [ ] **Indexed loop in `CheckForCircularReference` with custom comparer** — `src/Argon/Serialization/JsonSerializerInternalWriter.cs:269-271`. + `serializeStack.Contains(value, Serializer.EqualityComparer)` is LINQ `Enumerable.Contains` (boxed enumerator per value); replace with a `for` loop. + +- [ ] **One-time contract creation: duplicate reflection scan + O(n²) `Contains`** — `src/Argon/Serialization/DefaultContractResolver.cs:101-133, 144`. + `GetFieldsAndProperties` runs twice and `defaultMembers.Contains(member)` is linear per member. First-use latency only (result is cached); use a `HashSet` opportunistically. + +## LINQ-to-JSON / JSONPath + +### High priority — done + +- [x] **Replace LINQ `Cast()` in `JObject.Properties()`** — [JObject.cs](src/Argon/Linq/JObject.cs). + Now an iterator over `properties.InnerList` (the pattern `GetEnumerator()` already used), so there is no LINQ wrapper enumerable and no boxed interface enumerator. `CopyTo` got the same treatment. **1.3× faster, 27% less allocation.** + +- [x] **Route `JTokenWriter.WriteValue(int)` through `BoxedPrimitives`** — [JTokenWriter.cs](src/Argon/Linq/JTokenWriter.cs). + `int` is the most common CLR type for a JSON number and was the only numeric overload still boxing at the call site. Deliberately not applied to `short`/`ushort`/`byte`/`sbyte`/`uint`: those widen to `int`, which would change the CLR type stored in `JValue.Value` and so is a behaviour change, not just a perf one. **17% less allocation** on `IntArrayFromObject`. + +- [x] **Store a `Regex` instance in JSONPath `=~` expressions** — [BooleanQueryExpression.cs](src/Argon.JsonPath/BooleanQueryExpression.cs). + Evaluation went through static `Regex.IsMatch` per candidate token — a process-wide cache probe each time, degrading to a full pattern re-parse once more than `Regex.CacheSize` patterns are in play. The constructed `Regex` is now cached with the timeout it was built for, in a single reference field so a concurrently evaluated cached `JPath` cannot observe a regex paired with the wrong timeout. Built lazily so an invalid pattern still surfaces during evaluation rather than at parse time. + +- [x] **Override `GetItem` in `JArray`** — [JArray.cs](src/Argon/Linq/JArray.cs). + Indexes the backing list directly instead of going through the virtual `ChildrenTokens` property and an `IList` interface dispatch, mirroring what `IndexOfItem` already did. + +### Medium priority + +- [ ] **Indexed loops in `ClearItems` / `CopyItemsTo` / `ContentsHashCode`** — `src/Argon/Linq/JContainer.cs:315-327, 361, 614-623`. + `foreach` over interface-typed `children` boxes an enumerator per container; `ContentsHashCode` recurses over whole trees via `JTokenEqualityComparer.GetHashCode`. Use the indexed-loop pattern already used in the copy constructor (lines 26-32). + +- [ ] **Special-case `JArray`/`JObject` iteration in JSONPath filters** — `src/Argon.JsonPath/ArrayIndexFilter.cs:12-17`, `src/Argon.JsonPath/QueryFilter.cs:8-16`. + `foreach (var v in t)` routes through `Children()` → `JEnumerable` over interface-typed lists (boxed enumerator per input token). `ArrayIndexFilter` already pattern-matches `JArray` — bind it and index the backing list. + +- [ ] **Skip triple dictionary hash per property in `JTokenWriter.WritePropertyName`** — `src/Argon/Linq/JTokenWriter.cs:120-131`, `src/Argon/Linq/JObject.cs:104`. + `Remove(name)` + `ValidateToken`'s `Contains(name)` + `AddKey` = three hashes per property on the `FromObject` path. The writer path already flows through `AddAndSkipParentCheck`; let `JObject.InsertItem` skip the duplicate-name `Contains` when that flag is set (the preceding `Remove` guarantees uniqueness). + +- [ ] **Iterate `InnerList` in `JObject.CopyTo` (KVP)** — `src/Argon/Linq/JObject.cs:503-510`. + Boxed enumerator + per-item cast; iterate `properties.InnerList` as `GetEnumerator()` does. + +### Low priority + +- [ ] **Span-based JSONPath parse for numbers and escape-free strings** — `src/Argon.JsonPath/JPath.cs:563-590, 626-684`. + `TryParseValue` accumulates digits into a `StringBuilder` before parsing (parse the `expression.AsSpan(start, length)` slice instead); `ReadQuotedString` allocates a `StringBuilder` even with no escapes (defer until first `\`, else `Substring`). Mitigated by the path cache, so parse-time only. + +- [ ] **(Awareness only) `JToken.Path` is O(depth × width)** — `src/Argon/Linq/JToken.cs:197-240`. + Per array ancestor it does a linear `IndexOf(previous)`; building paths for every element of a big array is quadratic. A real fix needs per-child indices (invasive; matches Newtonsoft behavior as-is). + +## Incidental changes made while implementing the above + +- **Unblocked the test build** — [AssemblyInfo.cs](src/ArgonTests/AssemblyInfo.cs). + `[assembly: CollectionBehavior(DisableTestParallelization = true)]` became a build error (obsolete as error) after the xUnit v3 4.0.0 bump, so no tests could run at all. Replaced with the current API, `[assembly: Xunit.v3.Parallelization(Mode = Xunit.Sdk.ParallelMode.None)]`. + +- **New tests for the deferred property name** — [JsonTextWriterTest.cs](src/ArgonTests/JsonTextWriterTest.cs). + `PathWithSpanPropertyNames` and `SpanPropertyNameInExceptionPath`. The span `WritePropertyName` change is the only one that alters how a value is stored and read back later, and there was no existing coverage pairing the span overload with `Path`. They cover names changing length at one depth, names surviving a push/pop, names needing path escaping, a span sliced out of a larger buffer, and mixing the string and span overloads. + +- **Fixed a pre-existing test failure** — [XmlNodeConverterTest.cs](src/ArgonTests/Converters/XmlNodeConverterTest.cs). + `FloatParseHandlingDecimal` failed on both net10.0 and net48, before and after these changes. It built its input as `(decimal) Math.PI + 1000000000m`, but the `double` → `decimal` conversion returns full precision (`3.1415926535897931159979634685`) rather than the 15 significant digits its hardcoded expectation was written for, so it had become a test of conversion precision rather than of the XML/JSON round trip. Confirmed with a standalone console app that no Argon code was involved in producing the differing value. Now uses the decimal literal `1000000003.14159265358979m` directly, so it tests what it intends to; both the XML assertion and the round-trip assertion pass. + + Full suite is green: **2358/2358 on net10.0, 2337/2337 on net48, 9/9 F#**. + +## Already optimal (checked, no action) + +- Write-side escape scanning uses `SearchValues` with a ≥16-char threshold; `JsonTextWriter` uses `TryFormat` into pooled buffers; `BoxedPrimitives` covers `JValue(long/bool/double/decimal)`. +- `StringBuffer`/`BufferUtils`/reader `charBuffer` are ArrayPool-backed; `Base64Encoder` has a stackalloc net6+ path; `ConvertUtils.GetTypeCode` uses `FrozenDictionary`. +- Contract property names are interned via `DefaultJsonNameTable`; presence dictionaries have capacity hints; `EnumUtils` caches per (enum, naming strategy) with a struct key. +- `JPropertyKeyedCollection` lookups are dictionary-backed; `ScanFilter` descendants use an allocation-free pointer walk; JPath parses are cached via `JTokenExtensions.ParsePath`. From 9b47126ac33dc4d1d44fbf547856753c42952025 Mon Sep 17 00:00:00 2001 From: Simon Cropp Date: Thu, 27 Aug 2026 18:59:38 +1000 Subject: [PATCH 02/12] refs or cleanup --- .github/ISSUE_TEMPLATE/bug_report.md | 2 +- src/Argon.JsonPath/ArrayIndexFilter.cs | 10 +++--- src/Argon.JsonPath/JPath.cs | 29 +++++++++++---- src/Argon.JsonPath/QueryFilter.cs | 14 ++++++-- src/Argon/JsonConvert.cs | 10 ++++-- src/Argon/JsonReader.cs | 7 ++-- src/Argon/JsonTextReader.cs | 8 +++-- src/Argon/Linq/JContainer.cs | 30 ++++++++++------ src/Argon/Linq/JObject.cs | 7 +++- src/Argon/NamingStrategy/NamingStrategy.cs | 36 +++++++++++++++++-- .../Serialization/DefaultContractResolver.cs | 8 +++-- .../JsonSerializerInternalReader.cs | 29 +++++++++++++-- .../JsonSerializerInternalWriter.cs | 34 ++++++++++++------ src/Argon/Utilities/ConvertUtils.cs | 12 ++++--- src/Argon/Utilities/DateTimeUtils.cs | 30 +++++++++++----- src/Argon/Utilities/JavaScriptUtils.cs | 25 ++++++++++++- 16 files changed, 225 insertions(+), 66 deletions(-) diff --git a/.github/ISSUE_TEMPLATE/bug_report.md b/.github/ISSUE_TEMPLATE/bug_report.md index f48453937..55d3f400b 100644 --- a/.github/ISSUE_TEMPLATE/bug_report.md +++ b/.github/ISSUE_TEMPLATE/bug_report.md @@ -33,4 +33,4 @@ Ensure you have replicated the bug in a minimal solution with the fewest moving #### Submit a PR that fixes the bug -Submit a [Pull Request (PR)](https://help.github.com/articles/about-pull-requests/) that fixes the bug. Include in this PR a test that verifies the fix. If you were not able to fix the bug, a PR that illustrates your partial progress will suffice. \ No newline at end of file +Submit a [Pull Request (PR)](https://help.github.com/articles/about-pull-requests/) that fixes the bug. Include in this PR a test that verifies the fix. If you were not able to fix the bug, a PR that illustrates your partial progress will suffice. diff --git a/src/Argon.JsonPath/ArrayIndexFilter.cs b/src/Argon.JsonPath/ArrayIndexFilter.cs index aad799bea..03de7bdc7 100644 --- a/src/Argon.JsonPath/ArrayIndexFilter.cs +++ b/src/Argon.JsonPath/ArrayIndexFilter.cs @@ -1,4 +1,4 @@ -class ArrayIndexFilter : +class ArrayIndexFilter : PathFilter { public int? Index { get; set; } @@ -9,11 +9,13 @@ public override IEnumerable ExecuteFilter(JToken root, IEnumerable ExecuteFilter(JToken root, IEnumerable /// /// Converts the to its JSON string representation. /// - public static string ToString(char value) => - ToString(new[]{value}.AsSpan()); + public static string ToString(char value) + { + Span chars = stackalloc char[1]; + chars[0] = value; + return ToString(chars); + } /// /// Converts the to its JSON string representation. diff --git a/src/Argon/JsonReader.cs b/src/Argon/JsonReader.cs index 972b17972..b994c184f 100644 --- a/src/Argon/JsonReader.cs +++ b/src/Argon/JsonReader.cs @@ -1,4 +1,4 @@ -// Copyright (c) 2007 James Newton-King. All rights reserved. +// Copyright (c) 2007 James Newton-King. All rights reserved. // Use of this source code is governed by The MIT License, // as found in the license.md file. @@ -757,7 +757,7 @@ bool ReadArrayElementIntoByteArrayReportDone(List buffer) return d; } - if (ConvertUtils.DecimalTryParse(s.ToCharArray(), 0, s.Length, out d) == ParseResult.Success) + if (ConvertUtils.DecimalTryParse(s.AsSpan(), 0, s.Length, out d) == ParseResult.Success) { // This is to handle strings like "96.014e-05" that are not supported by traditional decimal.TryParse SetToken(d); @@ -783,8 +783,7 @@ bool ReadArrayElementIntoByteArrayReportDone(List buffer) } // fallback handles strings like "96.014e-05" not supported by decimal.TryParse - var chars = s.ToArray(); - if (ConvertUtils.DecimalTryParse(chars, 0, chars.Length, out d) == ParseResult.Success) + if (ConvertUtils.DecimalTryParse(s, 0, s.Length, out d) == ParseResult.Success) { SetToken(d); return d; diff --git a/src/Argon/JsonTextReader.cs b/src/Argon/JsonTextReader.cs index 8bf8fa052..00ed47e45 100644 --- a/src/Argon/JsonTextReader.cs +++ b/src/Argon/JsonTextReader.cs @@ -1,4 +1,4 @@ -// Copyright (c) 2007 James Newton-King. All rights reserved. +// Copyright (c) 2007 James Newton-King. All rights reserved. // Use of this source code is governed by The MIT License, // as found in the license.md file. @@ -95,7 +95,7 @@ void ParseReadString(char quote, ReadType readType) data = []; } else if (stringReference.Length == 36 && - ConvertUtils.TryConvertGuid(stringReference.ToString(), out var g)) + ConvertUtils.TryConvertGuid(stringReference.AsSpan(), out var g)) { data = g.ToByteArray(); } @@ -137,8 +137,10 @@ void ShiftBufferIfNeeded() // once in the last 10% of the buffer, or buffer is already very large then // shift the remaining content to the start to avoid unnecessarily increasing // the buffer size when reading numbers/strings + // integer math: this runs once per string/number token, and the double conversion and + // multiply it replaces showed up next to the actual copy var length = charBuffer.Length; - if (length - charPos <= length * 0.1 || length >= LargeBufferLength) + if ((length - charPos) * 10L <= length || length >= LargeBufferLength) { var count = charsUsed - charPos; if (count > 0) diff --git a/src/Argon/Linq/JContainer.cs b/src/Argon/Linq/JContainer.cs index c5b19d3fc..23cc9d450 100644 --- a/src/Argon/Linq/JContainer.cs +++ b/src/Argon/Linq/JContainer.cs @@ -1,4 +1,4 @@ -// Copyright (c) 2007 James Newton-King. All rights reserved. +// Copyright (c) 2007 James Newton-King. All rights reserved. // Use of this source code is governed by The MIT License, // as found in the license.md file. @@ -207,7 +207,9 @@ internal virtual bool InsertItem(int index, JToken? item, bool skipParentCheck) // haven't inserted new token yet so next token is still at the inserting index var next = index == children.Count ? null : children[index]; - ValidateToken(item, null); + // skipParentCheck is only set by the writer, which has already removed any property + // with the same name, so JObject can skip hashing the name a second time + ValidateToken(item, null, skipParentCheck); item.Parent = this; @@ -292,7 +294,7 @@ internal virtual void SetItem(int index, JToken? item) item = EnsureParentToken(item, false); - ValidateToken(item, existing); + ValidateToken(item, existing, false); var previous = index == 0 ? null : children[index - 1]; var next = index == children.Count - 1 ? null : children[index + 1]; @@ -314,10 +316,12 @@ internal virtual void SetItem(int index, JToken? item) internal virtual void ClearItems() { + // indexed loop: children is interface typed, so foreach boxes an enumerator var children = ChildrenTokens; - foreach (var item in children) + for (var i = 0; i < children.Count; i++) { + var item = children[i]; item.Parent = null; item.Previous = null; item.Next = null; @@ -357,11 +361,10 @@ internal virtual void CopyItemsTo(Array array, int arrayIndex) throw new ArgumentException("The number of elements in the source JObject is greater than the available space from arrayIndex to the end of the destination array."); } - var index = 0; - foreach (var token in ChildrenTokens) + var children = ChildrenTokens; + for (var i = 0; i < children.Count; i++) { - array.SetValue(token, arrayIndex + index); - index++; + array.SetValue(children[i], arrayIndex + i); } } @@ -381,7 +384,11 @@ internal static bool IsTokenUnchanged(JToken currentValue, JToken? newValue) return false; } - internal virtual void ValidateToken(JToken o, JToken? existing) + /// + /// Set when the caller guarantees the token can not collide with an existing one. Only + /// acts on it, to skip its duplicate property name check. + /// + internal virtual void ValidateToken(JToken o, JToken? existing, bool skipDuplicateNameCheck) { if (o.Type == JTokenType.Property) { @@ -614,9 +621,10 @@ static JProperty ReadProperty(JsonReader reader, JsonLoadSettings? settings, IJs internal int ContentsHashCode() { var hashCode = 0; - foreach (var item in ChildrenTokens) + var children = ChildrenTokens; + for (var i = 0; i < children.Count; i++) { - hashCode ^= item.GetDeepHashCode(); + hashCode ^= children[i].GetDeepHashCode(); } return hashCode; diff --git a/src/Argon/Linq/JObject.cs b/src/Argon/Linq/JObject.cs index 8a44a5bc1..997ecd1bc 100644 --- a/src/Argon/Linq/JObject.cs +++ b/src/Argon/Linq/JObject.cs @@ -82,13 +82,18 @@ internal override bool InsertItem(int index, JToken? item, bool skipParentCheck) return base.InsertItem(index, item, skipParentCheck); } - internal override void ValidateToken(JToken o, JToken? existing) + internal override void ValidateToken(JToken o, JToken? existing, bool skipDuplicateNameCheck) { if (o.Type != JTokenType.Property) { throw new ArgumentException($"Can not add {o.GetType()} to {GetType()}."); } + if (skipDuplicateNameCheck) + { + return; + } + var newProperty = (JProperty) o; if (existing != null) diff --git a/src/Argon/NamingStrategy/NamingStrategy.cs b/src/Argon/NamingStrategy/NamingStrategy.cs index 7d477784a..b13705f9a 100644 --- a/src/Argon/NamingStrategy/NamingStrategy.cs +++ b/src/Argon/NamingStrategy/NamingStrategy.cs @@ -9,6 +9,12 @@ namespace Argon; /// public abstract class NamingStrategy { + // dictionary keys are user data, so the cache is capped; once it is full later keys are + // resolved without being added + const int dictionaryKeyCacheMax = 512; + + ConcurrentDictionary? dictionaryKeyCache; + /// /// A flag indicating whether dictionary keys should be processed. /// Defaults to false. @@ -45,14 +51,40 @@ public virtual string GetPropertyName(string name, bool hasSpecifiedName) /// The serialized dictionary key. public virtual string GetDictionaryKey(string name, object original) { - if (ProcessDictionaryKeys) + if (!ProcessDictionaryKeys) + { + return name; + } + + if (!CacheDictionaryKeys) { return ResolvePropertyName(name); } - return name; + // the same keys usually repeat across the entries of a dictionary, and across calls + var cache = dictionaryKeyCache ??= new(); + if (cache.TryGetValue(name, out var resolved)) + { + return resolved; + } + + resolved = ResolvePropertyName(name); + if (cache.Count < dictionaryKeyCacheMax) + { + cache.TryAdd(name, resolved); + } + + return resolved; } + /// + /// A flag indicating whether resolved dictionary keys can be cached, which requires + /// to return the same result every time it is passed the same + /// name. True for all the naming strategies included in Argon. Override to false in a + /// strategy that resolves a name differently depending on state outside that name. + /// + protected virtual bool CacheDictionaryKeys => true; + /// /// Resolves the specified property name. /// diff --git a/src/Argon/Serialization/DefaultContractResolver.cs b/src/Argon/Serialization/DefaultContractResolver.cs index 37d340856..2920e5a98 100644 --- a/src/Argon/Serialization/DefaultContractResolver.cs +++ b/src/Argon/Serialization/DefaultContractResolver.cs @@ -102,8 +102,10 @@ protected virtual IEnumerable GetSerializableMembers(Type type) var dataContractAttribute = JsonTypeReflector.GetDataContractAttribute(type); // Exclude index properties and ByRef types - var defaultMembers = type.GetFieldsAndProperties(BindingFlags.Instance | BindingFlags.Public) - .Where(FilterMembers).ToList(); + // HashSet: ShouldSerialize probes this for every member of the type, and a list probe is linear + var defaultMembers = new HashSet( + type.GetFieldsAndProperties(BindingFlags.Instance | BindingFlags.Public) + .Where(FilterMembers)); // Do not filter ByRef types here because accessing FieldType/PropertyType can trigger additional assembly loads foreach (var member in type.GetFieldsAndProperties(BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Instance)) @@ -132,7 +134,7 @@ protected virtual IEnumerable GetSerializableMembers(Type type) return serializableMembers; } - bool ShouldSerialize(MemberInfo member, List defaultMembers, DataContractAttribute? dataContractAttribute) + bool ShouldSerialize(MemberInfo member, HashSet defaultMembers, DataContractAttribute? dataContractAttribute) { // exclude members that are compiler generated if set if (!SerializeCompilerGeneratedMembers && diff --git a/src/Argon/Serialization/JsonSerializerInternalReader.cs b/src/Argon/Serialization/JsonSerializerInternalReader.cs index 0254df505..527aeb872 100644 --- a/src/Argon/Serialization/JsonSerializerInternalReader.cs +++ b/src/Argon/Serialization/JsonSerializerInternalReader.cs @@ -1,4 +1,4 @@ -// Copyright (c) 2007 James Newton-King. All rights reserved. +// Copyright (c) 2007 James Newton-King. All rights reserved. // Use of this source code is governed by The MIT License, // as found in the license.md file. @@ -34,6 +34,31 @@ enum PropertyPresence JsonContract GetContract(Type type) => Serializer.ResolveContract(type); + // type names are untrusted input, so the cache is capped; once it is full later names are + // split without being added + const int typeNameCacheMax = 128; + + Dictionary? typeNameKeys; + + // the same $type repeats for every item of a polymorphic collection, and splitting it + // allocates a substring for the type and one for the assembly every time + TypeNameKey SplitTypeName(string qualifiedTypeName) + { + var keys = typeNameKeys ??= new(); + if (keys.TryGetValue(qualifiedTypeName, out var key)) + { + return key; + } + + key = ReflectionUtils.SplitFullyQualifiedTypeName(qualifiedTypeName); + if (keys.Count < typeNameCacheMax) + { + keys.Add(qualifiedTypeName, key); + } + + return key; + } + [RequiresUnreferencedCode(MiscellaneousUtils.TrimWarning)] [RequiresDynamicCode(MiscellaneousUtils.AotWarning)] public object? Deserialize(JsonReader reader, Type? type, bool? checkAdditionalContent) @@ -650,7 +675,7 @@ void ResolveTypeName(JsonReader reader, ref Type? type, ref JsonContract? contra if (resolvedTypeNameHandling != TypeNameHandling.None) { - var typeNameKey = ReflectionUtils.SplitFullyQualifiedTypeName(qualifiedTypeName); + var typeNameKey = SplitTypeName(qualifiedTypeName); var binder = Serializer.SerializationBinder ?? DefaultSerializationBinder.Instance; Type specifiedType; diff --git a/src/Argon/Serialization/JsonSerializerInternalWriter.cs b/src/Argon/Serialization/JsonSerializerInternalWriter.cs index 057ec787a..29659d856 100644 --- a/src/Argon/Serialization/JsonSerializerInternalWriter.cs +++ b/src/Argon/Serialization/JsonSerializerInternalWriter.cs @@ -1,4 +1,4 @@ -// Copyright (c) 2007 James Newton-King. All rights reserved. +// Copyright (c) 2007 James Newton-King. All rights reserved. // Use of this source code is governed by The MIT License, // as found in the license.md file. @@ -14,6 +14,26 @@ class JsonSerializerInternalWriter(JsonSerializer serializer) : int rootLevel; readonly List serializeStack = []; + bool SerializeStackContains(object value) + { + var comparer = Serializer.EqualityComparer; + if (comparer == null) + { + return serializeStack.Contains(value); + } + + // indexed loop: Enumerable.Contains boxes an enumerator for every value serialized + for (var i = 0; i < serializeStack.Count; i++) + { + if (comparer.Equals(serializeStack[i], value)) + { + return true; + } + } + + return false; + } + // formatted $type names, cached for the duration of this serialization. building one // concatenates the type and assembly names and then re-parses the result through // RemoveAssemblyDetails, allocating a StringBuilder and a string, and polymorphic payloads @@ -273,9 +293,7 @@ JsonContractType.Primitive or referenceLoopHandling = containerContract.ItemReferenceLoopHandling; } - var exists = Serializer.EqualityComparer == null - ? serializeStack.Contains(value) - : serializeStack.Contains(value, Serializer.EqualityComparer); + var exists = SerializeStackContains(value); if (!exists) { @@ -1061,16 +1079,12 @@ static string GetDictionaryPropertyName(object key, JsonContract contract, out b var dt = (DateTime) key; escape = false; - var writer = new StringWriter(InvariantCulture); - DateTimeUtils.WriteDateTimeString(writer, dt); - return writer.ToString(); + return DateTimeUtils.ToDateTimeString(dt); } case PrimitiveTypeCode.DateTimeOffset: { escape = false; - var writer = new StringWriter(InvariantCulture); - DateTimeUtils.WriteDateTimeOffsetString(writer, (DateTimeOffset) key); - return writer.ToString(); + return DateTimeUtils.ToDateTimeOffsetString((DateTimeOffset) key); } case PrimitiveTypeCode.Double: { diff --git a/src/Argon/Utilities/ConvertUtils.cs b/src/Argon/Utilities/ConvertUtils.cs index a8ab52ea2..79cb8ce93 100644 --- a/src/Argon/Utilities/ConvertUtils.cs +++ b/src/Argon/Utilities/ConvertUtils.cs @@ -1,4 +1,4 @@ -// Copyright (c) 2007 James Newton-King. All rights reserved. +// Copyright (c) 2007 James Newton-King. All rights reserved. // Use of this source code is governed by The MIT License, // as found in the license.md file. @@ -548,7 +548,7 @@ PrimitiveTypeCode.Int64 or PrimitiveTypeCode.Int64Nullable or _ => false }; - public static ParseResult Int32TryParse(char[] chars, int start, int length, out int value) + public static ParseResult Int32TryParse(CharSpan chars, int start, int length, out int value) { value = 0; @@ -642,7 +642,7 @@ public static ParseResult Int32TryParse(char[] chars, int start, int length, out return ParseResult.Success; } - public static ParseResult Int64TryParse(char[] chars, int start, int length, out long value) + public static ParseResult Int64TryParse(CharSpan chars, int start, int length, out long value) { value = 0; @@ -734,7 +734,7 @@ public static ParseResult Int64TryParse(char[] chars, int start, int length, out return ParseResult.Success; } - public static ParseResult DecimalTryParse(char[] chars, int start, int length, out decimal value) + public static ParseResult DecimalTryParse(CharSpan chars, int start, int length, out decimal value) { value = 0M; const decimal decimalMaxValueHi28 = 7922816251426433759354395033M; @@ -987,6 +987,10 @@ public static bool TryConvertGuid(string s, out Guid g) => // GUID has to have format 00000000-0000-0000-0000-000000000000 Guid.TryParseExact(s, "D", out g); + public static bool TryConvertGuid(CharSpan s, out Guid g) => + // GUID has to have format 00000000-0000-0000-0000-000000000000 + Guid.TryParseExact(s, "D", out g); + public static bool TryHexTextToInt(char[] text, int start, int end, out int value) { value = 0; diff --git a/src/Argon/Utilities/DateTimeUtils.cs b/src/Argon/Utilities/DateTimeUtils.cs index e0e608884..a7879dbed 100644 --- a/src/Argon/Utilities/DateTimeUtils.cs +++ b/src/Argon/Utilities/DateTimeUtils.cs @@ -135,12 +135,19 @@ internal static bool TryParseDateTimeOffset(string s, out DateTimeOffset dt) internal static void WriteDateTimeString(TextWriter writer, DateTime value) { - var chars = new char[64]; + Span chars = stackalloc char[64]; var pos = WriteDateTimeString(chars, 0, value, null, value.Kind); - writer.Write(chars, 0, pos); + writer.Write(chars[..pos]); } - internal static int WriteDateTimeString(char[] chars, int start, DateTime value, TimeSpan? offset, DateTimeKind kind) + internal static string ToDateTimeString(DateTime value) + { + Span chars = stackalloc char[64]; + var pos = WriteDateTimeString(chars, 0, value, null, value.Kind); + return chars[..pos].ToString(); + } + + internal static int WriteDateTimeString(Span chars, int start, DateTime value, TimeSpan? offset, DateTimeKind kind) { var pos = WriteDefaultIsoDate(chars, start, value); @@ -157,7 +164,7 @@ internal static int WriteDateTimeString(char[] chars, int start, DateTime value, return pos; } - static int WriteDefaultIsoDate(char[] chars, int start, DateTime dt) + static int WriteDefaultIsoDate(Span chars, int start, DateTime dt) { var length = 19; @@ -195,7 +202,7 @@ static int WriteDefaultIsoDate(char[] chars, int start, DateTime dt) return start + length; } - static void CopyIntToCharArray(char[] chars, int start, int value, int digits) + static void CopyIntToCharArray(Span chars, int start, int value, int digits) { while (digits-- != 0) { @@ -204,7 +211,7 @@ static void CopyIntToCharArray(char[] chars, int start, int value, int digits) } } - internal static int WriteDateTimeOffset(char[] chars, int start, TimeSpan offset) + internal static int WriteDateTimeOffset(Span chars, int start, TimeSpan offset) { chars[start++] = offset.Ticks >= 0L ? '+' : '-'; @@ -223,10 +230,17 @@ internal static int WriteDateTimeOffset(char[] chars, int start, TimeSpan offset internal static void WriteDateTimeOffsetString(TextWriter writer, DateTimeOffset value) { - var chars = new char[64]; + Span chars = stackalloc char[64]; var pos = WriteDateTimeString(chars, 0, value.DateTime, value.Offset, DateTimeKind.Local); - writer.Write(chars, 0, pos); + writer.Write(chars[..pos]); + } + + internal static string ToDateTimeOffsetString(DateTimeOffset value) + { + Span chars = stackalloc char[64]; + var pos = WriteDateTimeString(chars, 0, value.DateTime, value.Offset, DateTimeKind.Local); + return chars[..pos].ToString(); } #endregion diff --git a/src/Argon/Utilities/JavaScriptUtils.cs b/src/Argon/Utilities/JavaScriptUtils.cs index 1d9b2302b..2e614f115 100644 --- a/src/Argon/Utilities/JavaScriptUtils.cs +++ b/src/Argon/Utilities/JavaScriptUtils.cs @@ -1,4 +1,4 @@ -// Copyright (c) 2007 James Newton-King. All rights reserved. +// Copyright (c) 2007 James Newton-King. All rights reserved. // Use of this source code is governed by The MIT License, // as found in the license.md file. @@ -294,6 +294,29 @@ public static string ToEscapedJavaScriptString(CharSpan value, char delimiter, b { var escapeFlags = GetCharEscapeFlags(escapeHandling, delimiter); + if (FirstCharToEscape(value, escapeFlags, escapeHandling) == -1) + { + // nothing to escape: build the result directly rather than through a StringBuilder + if (!appendDelimiters) + { + return value.ToString(); + } + + var length = value.Length + 2; + var quoted = BufferUtils.RentBuffer(length); + try + { + quoted[0] = delimiter; + value.CopyTo(quoted.AsSpan(1)); + quoted[length - 1] = delimiter; + return new(quoted, 0, length); + } + finally + { + BufferUtils.ReturnBuffer(quoted); + } + } + // size for the delimiters too, otherwise the StringBuilder always grows on the first write using var w = StringUtils.CreateStringWriter(value.Length + (appendDelimiters ? 2 : 0)); char[]? buffer = null; From d5723546ba615ed4d64a4766f8d84877da02cf76 Mon Sep 17 00:00:00 2001 From: Simon Cropp Date: Thu, 27 Aug 2026 20:20:49 +1000 Subject: [PATCH 03/12] Span based JSONPath parsing, benchmarks and results for the medium and low priority perf items JPath: parse numbers from a slice of the expression rather than accumulating them into a StringBuilder, and return an unescaped quoted string as a Substring, only building a StringBuilder from the first escape on. The span number parsers need Polyfill, which Argon.JsonPath does not reference, so TryParseInt64 and TryParseDouble fall back to a string on net4x. Tests for the quoted string parse: text between two escapes, an escape as the final character, and the empty string. The existing tests only covered a single escape or none. Benchmarks for every medium and low priority item, and the measured results in todo.md. The timings carry a caveat: the machine alternated between two performance modes roughly 2x apart during the session, so runs were ordered ABBA and only differences that hold within a mode are reported as wins. Allocation numbers are deterministic and unaffected. Also strips BOMs that earlier edits added to nine files. .editorconfig sets charset = utf-8 for *.cs, and none of these files had one before. --- src/Argon.JsonPath/JPath.cs | 22 +- src/Argon/JsonConvert.cs | 2 +- src/Argon/JsonReader.cs | 2 +- src/Argon/JsonTextReader.cs | 2 +- src/Argon/Linq/JContainer.cs | 2 +- .../JsonSerializerInternalReader.cs | 2 +- .../JsonSerializerInternalWriter.cs | 2 +- src/Argon/Utilities/ConvertUtils.cs | 2 +- src/Argon/Utilities/JavaScriptUtils.cs | 2 +- .../Benchmarks/PerValueBenchmarks.cs | 311 ++++++++++++++++++ .../Linq/JsonPath/JPathParseTests.cs | 31 ++ src/Benchmark.Tests/Program.cs | 9 +- todo.md | 131 +++++--- 13 files changed, 465 insertions(+), 55 deletions(-) create mode 100644 src/ArgonTests/Benchmarks/PerValueBenchmarks.cs diff --git a/src/Argon.JsonPath/JPath.cs b/src/Argon.JsonPath/JPath.cs index 634dfae3f..8e8110c86 100644 --- a/src/Argon.JsonPath/JPath.cs +++ b/src/Argon.JsonPath/JPath.cs @@ -1,4 +1,4 @@ -// Copyright (c) 2007 James Newton-King. All rights reserved. +// Copyright (c) 2007 James Newton-King. All rights reserved. // Use of this source code is governed by The MIT License, // as found in the license.md file. @@ -574,13 +574,13 @@ bool TryParseValue(out object? value) if (numberText.IndexOfAny(floatCharacters) == -1) { - var result = long.TryParse(numberText, NumberStyles.Integer, InvariantCulture, out var l); + var result = TryParseInt64(numberText, out var l); value = l; return result; } else { - var result = double.TryParse(numberText, NumberStyles.Float | NumberStyles.AllowThousands, InvariantCulture, out var d); + var result = TryParseDouble(numberText, out var d); value = d; return result; } @@ -623,6 +623,22 @@ bool TryParseValue(out object? value) return false; } + // the span overloads of the number parsers need Polyfill on the old frameworks, and this + // project does not reference it, so those keep paying for a string +#if NET6_0_OR_GREATER + static bool TryParseInt64(CharSpan text, out long value) => + long.TryParse(text, NumberStyles.Integer, InvariantCulture, out value); + + static bool TryParseDouble(CharSpan text, out double value) => + double.TryParse(text, NumberStyles.Float | NumberStyles.AllowThousands, InvariantCulture, out value); +#else + static bool TryParseInt64(CharSpan text, out long value) => + long.TryParse(text.ToString(), NumberStyles.Integer, InvariantCulture, out value); + + static bool TryParseDouble(CharSpan text, out double value) => + double.TryParse(text.ToString(), NumberStyles.Float | NumberStyles.AllowThousands, InvariantCulture, out value); +#endif + string ReadQuotedString() { // the builder is only created once an escape is found: a string with no escapes in it diff --git a/src/Argon/JsonConvert.cs b/src/Argon/JsonConvert.cs index c060a3cd8..332c36455 100644 --- a/src/Argon/JsonConvert.cs +++ b/src/Argon/JsonConvert.cs @@ -1,4 +1,4 @@ -// Copyright (c) 2007 James Newton-King. All rights reserved. +// Copyright (c) 2007 James Newton-King. All rights reserved. // Use of this source code is governed by The MIT License, // as found in the license.md file. diff --git a/src/Argon/JsonReader.cs b/src/Argon/JsonReader.cs index b994c184f..c151542a2 100644 --- a/src/Argon/JsonReader.cs +++ b/src/Argon/JsonReader.cs @@ -1,4 +1,4 @@ -// Copyright (c) 2007 James Newton-King. All rights reserved. +// Copyright (c) 2007 James Newton-King. All rights reserved. // Use of this source code is governed by The MIT License, // as found in the license.md file. diff --git a/src/Argon/JsonTextReader.cs b/src/Argon/JsonTextReader.cs index 00ed47e45..3ecae55a6 100644 --- a/src/Argon/JsonTextReader.cs +++ b/src/Argon/JsonTextReader.cs @@ -1,4 +1,4 @@ -// Copyright (c) 2007 James Newton-King. All rights reserved. +// Copyright (c) 2007 James Newton-King. All rights reserved. // Use of this source code is governed by The MIT License, // as found in the license.md file. diff --git a/src/Argon/Linq/JContainer.cs b/src/Argon/Linq/JContainer.cs index 23cc9d450..8cfa76611 100644 --- a/src/Argon/Linq/JContainer.cs +++ b/src/Argon/Linq/JContainer.cs @@ -1,4 +1,4 @@ -// Copyright (c) 2007 James Newton-King. All rights reserved. +// Copyright (c) 2007 James Newton-King. All rights reserved. // Use of this source code is governed by The MIT License, // as found in the license.md file. diff --git a/src/Argon/Serialization/JsonSerializerInternalReader.cs b/src/Argon/Serialization/JsonSerializerInternalReader.cs index 527aeb872..6e469781a 100644 --- a/src/Argon/Serialization/JsonSerializerInternalReader.cs +++ b/src/Argon/Serialization/JsonSerializerInternalReader.cs @@ -1,4 +1,4 @@ -// Copyright (c) 2007 James Newton-King. All rights reserved. +// Copyright (c) 2007 James Newton-King. All rights reserved. // Use of this source code is governed by The MIT License, // as found in the license.md file. diff --git a/src/Argon/Serialization/JsonSerializerInternalWriter.cs b/src/Argon/Serialization/JsonSerializerInternalWriter.cs index 29659d856..5341761dd 100644 --- a/src/Argon/Serialization/JsonSerializerInternalWriter.cs +++ b/src/Argon/Serialization/JsonSerializerInternalWriter.cs @@ -1,4 +1,4 @@ -// Copyright (c) 2007 James Newton-King. All rights reserved. +// Copyright (c) 2007 James Newton-King. All rights reserved. // Use of this source code is governed by The MIT License, // as found in the license.md file. diff --git a/src/Argon/Utilities/ConvertUtils.cs b/src/Argon/Utilities/ConvertUtils.cs index 79cb8ce93..d22448bc3 100644 --- a/src/Argon/Utilities/ConvertUtils.cs +++ b/src/Argon/Utilities/ConvertUtils.cs @@ -1,4 +1,4 @@ -// Copyright (c) 2007 James Newton-King. All rights reserved. +// Copyright (c) 2007 James Newton-King. All rights reserved. // Use of this source code is governed by The MIT License, // as found in the license.md file. diff --git a/src/Argon/Utilities/JavaScriptUtils.cs b/src/Argon/Utilities/JavaScriptUtils.cs index 2e614f115..90e6d7447 100644 --- a/src/Argon/Utilities/JavaScriptUtils.cs +++ b/src/Argon/Utilities/JavaScriptUtils.cs @@ -1,4 +1,4 @@ -// Copyright (c) 2007 James Newton-King. All rights reserved. +// Copyright (c) 2007 James Newton-King. All rights reserved. // Use of this source code is governed by The MIT License, // as found in the license.md file. diff --git a/src/ArgonTests/Benchmarks/PerValueBenchmarks.cs b/src/ArgonTests/Benchmarks/PerValueBenchmarks.cs new file mode 100644 index 000000000..260b26650 --- /dev/null +++ b/src/ArgonTests/Benchmarks/PerValueBenchmarks.cs @@ -0,0 +1,311 @@ +// Copyright (c) 2007 James Newton-King. All rights reserved. +// Use of this source code is governed by The MIT License, +// as found in the license.md file. + +using BenchmarkDotNet.Attributes; + +// Benchmarks covering the medium and low priority items from the performance review in todo.md. +// The high priority items are covered by HotPathBenchmarks. + +// Dictionary keys pay for a conversion per entry per call: +// - DateTimeUtils.WriteDateTimeString allocated a 64 char array per call, and +// GetDictionaryPropertyName wrapped it in a StringWriter plus a StringBuilder to get a string +// back out. Both are gone: the date is formatted into a stackalloc buffer. +// - NamingStrategy.GetDictionaryKey re-ran the case conversion for every entry of every +// dictionary, and the same keys repeat across entries and across calls, so resolved keys are +// now cached on the strategy. +[MemoryDiagnoser] +public class DictionaryKeyBenchmark +{ + Dictionary dates; + Dictionary offsets; + Dictionary names; + JsonSerializerSettings camelCase; + + [GlobalSetup] + public void Setup() + { + var start = new DateTime(2020, 1, 1, 0, 0, 0, DateTimeKind.Utc); + dates = Enumerable.Range(0, 100) + .ToDictionary(_ => start.AddDays(_), _ => _); + offsets = Enumerable.Range(0, 100) + .ToDictionary(_ => new DateTimeOffset(start.AddDays(_)).ToOffset(TimeSpan.FromHours(10)), _ => _); + + // the same key set serialized repeatedly, which is what a cache can help with. The + // names are the ones a dictionary keyed on a domain concept tends to have + names = Enumerable.Range(0, 100) + .ToDictionary(_ => $"SomeDictionaryKey{_}", _ => _); + + camelCase = new() + { + ContractResolver = new DefaultContractResolver + { + NamingStrategy = new CamelCaseNamingStrategy + { + ProcessDictionaryKeys = true + } + } + }; + } + + [Benchmark] + public string SerializeDateKeys() => + JsonConvert.SerializeObject(dates); + + [Benchmark] + public string SerializeDateTimeOffsetKeys() => + JsonConvert.SerializeObject(offsets); + + [Benchmark] + public string SerializeCamelCaseKeys() => + JsonConvert.SerializeObject(names, camelCase); +} + +// JavaScriptUtils.ToEscapedJavaScriptString always built its result through a StringWriter over a +// StringBuilder, even for the common case of a string with nothing in it to escape. When the +// vectorized scan finds no char to escape the result is now copied out directly. +[MemoryDiagnoser] +public class EscapeFreeStringBenchmark +{ + string[] values; + + [GlobalSetup] + public void Setup() => + values = Enumerable.Range(0, 100) + .Select(_ => $"a typical value with no escapes in it at all {_}") + .ToArray(); + + [Benchmark] + public string ToStringNoEscapes() + { + string last = null; + foreach (var value in values) + { + last = JsonConvert.ToString(value); + } + + return last; + } +} + +// JsonSerializerInternalReader.ResolveTypeName split every $type it read into its type and +// assembly halves, allocating a substring for each. A polymorphic payload repeats the same few +// type names, so the split is now memoized for the duration of the deserialization. +[MemoryDiagnoser] +public class TypeNameReadBenchmark +{ + string json; + JsonSerializerSettings settings; + + [GlobalSetup] + public void Setup() + { + settings = new() + { + TypeNameHandling = TypeNameHandling.All + }; + + var items = Enumerable.Range(0, 200) + .Select(object (_) => new TypeNameItem + { + Name = $"item{_}", + Value = _ + }) + .ToArray(); + + json = JsonConvert.SerializeObject(items, settings); + } + + [Benchmark] + public object DeserializeWithTypeNames() => + JsonConvert.DeserializeObject(json, settings); + + public class TypeNameItem + { + public string Name { get; set; } + public int Value { get; set; } + } +} + +// Per value reader allocations: +// - ReadDecimalString copied its input to a char array before handing it to the fallback parser +// that handles exponent form decimals, which decimal.TryParse does not accept. The parsers +// only ever index their input, so they take spans now. +// - ReadAsBytes probes a 36 char string for a Guid before treating it as base64, and that probe +// materialized the string even when the answer was base64. +[MemoryDiagnoser] +public class ReadValueBenchmark +{ + string decimals; + string byteStrings; + + [GlobalSetup] + public void Setup() + { + // exponent form, which is the path that falls through decimal.TryParse to Argon's parser + decimals = $"[{string.Join(",", Enumerable.Range(1, 200).Select(_ => $"\"{_}6.014e-05\""))}]"; + + // 27 bytes base64 encodes to exactly 36 chars, the length of a Guid in D format, so + // every one of these takes the Guid probe before being decoded as base64 + var random = new byte[27]; + byteStrings = $"[{string.Join(",", Enumerable.Range(0, 200).Select(_ => + { + random[0] = (byte) _; + return $"\"{Convert.ToBase64String(random)}\""; + }))}]"; + } + + [Benchmark] + public decimal ReadExponentDecimals() + { + using var reader = new JsonTextReader(new StringReader(decimals)); + reader.Read(); + decimal total = 0; + // ReadAsDecimal on a string token is what reaches ReadDecimalString, and exponent form + // is what falls through decimal.TryParse to the parser that took the copy + while (reader.ReadAsDecimal() is { } value) + { + total += value; + } + + return total; + } + + [Benchmark] + public int ReadBase64Strings() + { + using var reader = new JsonTextReader(new StringReader(byteStrings)); + reader.Read(); + var total = 0; + while (reader.ReadAsBytes() is { } bytes) + { + total += bytes.Length; + } + + return total; + } +} + +// JsonTextReader.ReadNumberIntoBuffer walks a number a char at a time through a 28 case switch. +// This is here to decide whether finding the terminator with a vectorized IndexOfAnyExcept is +// worth it, given how short numbers usually are. +[MemoryDiagnoser] +public class NumberScanBenchmark +{ + string shortNumbers; + string longNumbers; + + [GlobalSetup] + public void Setup() + { + // the length JSON numbers usually are: ids, counts, small quantities + shortNumbers = $"[{string.Join(",", Enumerable.Range(0, 500).Select(_ => _ % 1000))}]"; + + // long enough for a vectorized scan to have something to do + longNumbers = $"[{string.Join(",", Enumerable.Range(0, 500).Select(_ => $"{_}.123456789012345"))}]"; + } + + [Benchmark] + public long ReadShortNumbers() => + Sum(shortNumbers); + + [Benchmark] + public long ReadLongNumbers() => + Sum(longNumbers); + + static long Sum(string json) + { + using var reader = new JsonTextReader(new StringReader(json)); + long count = 0; + while (reader.Read()) + { + if (reader.TokenType == JsonToken.Integer || reader.TokenType == JsonToken.Float) + { + count++; + } + } + + return count; + } +} + +// JTokenWriter.WritePropertyName removes any property of the same name before adding the new +// one, and the add then re-checked for a duplicate name before the keyed collection hashed the +// name a third time to index it. The check the writer's own remove already guarantees is skipped. +[MemoryDiagnoser] +public class JTokenPropertyWriteBenchmark +{ + WideModel model; + + [GlobalSetup] + public void Setup() => + model = new(); + + [Benchmark] + public JToken FromObjectWide() => + JToken.FromObject(model); + + public class WideModel + { + public string P01 { get; set; } = "one"; + public string P02 { get; set; } = "two"; + public string P03 { get; set; } = "three"; + public string P04 { get; set; } = "four"; + public int P05 { get; set; } = 5; + public int P06 { get; set; } = 6; + public int P07 { get; set; } = 7; + public int P08 { get; set; } = 8; + public bool P09 { get; set; } = true; + public bool P10 { get; set; } + public double P11 { get; set; } = 11.5; + public double P12 { get; set; } = 12.5; + public string P13 { get; set; } = "thirteen"; + public string P14 { get; set; } = "fourteen"; + public string P15 { get; set; } = "fifteen"; + public string P16 { get; set; } = "sixteen"; + } +} + +// JSONPath filters walked their input through JToken's enumerator, which goes through Children() +// and boxes an enumerator for every token the filter is handed. Both filters walk the container's +// children directly now. The path itself is parsed once and cached, but the parse also built a +// StringBuilder per number and per quoted string, which slicing the expression avoids. +[MemoryDiagnoser] +public class JsonPathFilterBenchmark +{ + JObject document; + int counter; + + [GlobalSetup] + public void Setup() => + document = new() + { + ["store"] = new JObject + { + ["book"] = new JArray( + Enumerable.Range(0, 200) + .Select(_ => new JObject + { + ["title"] = $"book{_}", + ["category"] = _ % 2 == 0 ? "fiction" : "reference", + ["price"] = _ % 40 + })) + } + }; + + [Benchmark] + public int QueryFilter() => + document.SelectTokens("$.store.book[?(@.price > 20)]").Count(); + + [Benchmark] + public int WildcardIndexFilter() => + document.SelectTokens("$.store.book[*].title").Count(); + + [Benchmark] + public int ParseAndQuery() + { + // a fresh path string each time, so the parse is not served from the path cache + counter++; + return document.SelectTokens($"$.store.book[?(@.category == 'fiction' && @.price > {counter % 20})]").Count(); + } +} diff --git a/src/ArgonTests/Linq/JsonPath/JPathParseTests.cs b/src/ArgonTests/Linq/JsonPath/JPathParseTests.cs index 581d81d73..341bd6e92 100644 --- a/src/ArgonTests/Linq/JsonPath/JPathParseTests.cs +++ b/src/ArgonTests/Linq/JsonPath/JPathParseTests.cs @@ -276,6 +276,37 @@ public void SinglePropertyAndFilterWithDoubleEscape() Assert.Equal("h\\i", (string) (JToken) expressions.Right); } + [Fact] + public void SinglePropertyAndFilterWithEscapesAroundText() + { + // the quoted string is sliced out of the expression until the first escape, and the + // text between escapes is copied in as each one is resolved + var path = new JPath(@"Blah[ ?( @.name=='one\ttwo\\three\nfour' ) ]"); + Assert.Equal(2, path.Filters.Count); + Assert.Equal("Blah", ((FieldFilter) path.Filters[0]).Name); + var expressions = (BooleanQueryExpression) ((QueryFilter) path.Filters[1]).Expression; + Assert.Equal(QueryOperator.Equals, expressions.Operator); + Assert.Equal("one\ttwo\\three\nfour", (string) (JToken) expressions.Right); + } + + [Fact] + public void SinglePropertyAndFilterWithEscapeAtEnd() + { + var path = new JPath(@"Blah[ ?( @.name=='hi\n' ) ]"); + Assert.Equal(2, path.Filters.Count); + var expressions = (BooleanQueryExpression) ((QueryFilter) path.Filters[1]).Expression; + Assert.Equal("hi\n", (string) (JToken) expressions.Right); + } + + [Fact] + public void SinglePropertyAndFilterWithEmptyString() + { + var path = new JPath(@"Blah[ ?( @.name=='' ) ]"); + Assert.Equal(2, path.Filters.Count); + var expressions = (BooleanQueryExpression) ((QueryFilter) path.Filters[1]).Expression; + Assert.Equal("", (string) (JToken) expressions.Right); + } + [Fact] public void SinglePropertyAndFilterWithRegexAndOptions() { diff --git a/src/Benchmark.Tests/Program.cs b/src/Benchmark.Tests/Program.cs index 5286cc242..f190bb626 100644 --- a/src/Benchmark.Tests/Program.cs +++ b/src/Benchmark.Tests/Program.cs @@ -37,7 +37,14 @@ public static void Main(string[] args) typeof(TypeNameWriteBenchmark), typeof(WideCreatorBenchmark), typeof(PopulateExistingBenchmark), - typeof(JTokenHotPathBenchmark) + typeof(JTokenHotPathBenchmark), + typeof(DictionaryKeyBenchmark), + typeof(EscapeFreeStringBenchmark), + typeof(TypeNameReadBenchmark), + typeof(ReadValueBenchmark), + typeof(JTokenPropertyWriteBenchmark), + typeof(JsonPathFilterBenchmark), + typeof(NumberScanBenchmark) ]); if (args.Length == 0) { diff --git a/todo.md b/todo.md index 7d7d7231e..6263947ec 100644 --- a/todo.md +++ b/todo.md @@ -3,9 +3,10 @@ Findings from a perf review of the core read/write path, serialization, and LINQ-to-JSON / JSONPath (2026-08-27). Prioritized within each section; high-priority items sit on per-character / per-token / per-property hot paths. -**All high-priority items are implemented.** Medium and low priority items remain open. +**All high, medium and low priority items are implemented**, except one that was explicitly +"benchmark first" and could not be measured reliably — see [Still open](#still-open). -## Measured impact +## Measured impact — high priority BenchmarkDotNet `--job short`, .NET 10.0.11, AMD Ryzen 9 5900X. Baseline is a clean worktree at the commit these changes sit on, running the identical benchmark code. Benchmarks live in @@ -35,6 +36,38 @@ Notes on the two rows that are not a clean win: overlap); the 17% allocation drop from routing `WriteValue(int)` through `BoxedPrimitives` is the real and repeatable part. +## Measured impact — medium and low priority + +Benchmarks in [PerValueBenchmarks.cs](src/ArgonTests/Benchmarks/PerValueBenchmarks.cs), same baseline +worktree and machine, `--job medium`. + +**Read the timings with the caveat below.** During this measurement session the machine alternated +between two performance modes roughly 2× apart: the *same* baseline binary measured 54.5 µs and +90.6 µs for `DeserializeWithTypeNames` in back-to-back runs, and a current-tree run measured 2.15 µs +then 1.00 µs for `FromObjectWide`. Runs were therefore ordered ABBA (current, baseline, baseline, +current) to expose the drift, and only differences that hold *within* a mode are reported as wins. +Allocation numbers are deterministic and are unaffected by any of this. + +| Benchmark | Allocated before → after | Time | +|---|---|---| +| `SerializeDateKeys` | 60.30 KB → 25.15 KB (**−58%**) | 14.75 µs → 6.66 µs (**~1.6–2.2× faster**) | +| `SerializeDateTimeOffsetKeys` | 62.85 KB → 27.70 KB (**−56%**) | 15.94 µs → 7.79 µs (**~1.4–2.0× faster**) | +| `ToStringNoEscapes` | 35.08 KB → 11.72 KB (**−67%**) | 4.31 µs → 1.79 µs (**~1.4–2.4× faster**) | +| `ReadBase64Strings` | 29.92 KB → 11.17 KB (**−63%**) | 10.68 µs → 7.94 µs (best of each; modes did not line up) | +| `ReadExponentDecimals` | 22.11 KB → 12.73 KB (**−42%**) | within noise | +| `DeserializeWithTypeNames` | 83.02 KB → 55.28 KB (**−33%**) | ~4% faster in both modes (90.6→87.0, 54.5→52.0) | +| `SerializeCamelCaseKeys` | 22.59 KB → 16.34 KB (**−28%**) | within noise in the one mode measured on both | +| `WildcardIndexFilter` | 448 B → 416 B | ~1% in mode; one boxed enumerator per input token gone | +| `QueryFilter` | 29176 B → 29136 B | not separable from noise | +| `ParseAndQuery` | 43712 B → 43672 B | not separable from noise | +| `FromObjectWide` | unchanged (4.84 KB) | not separable from noise (one of three name hashes per property gone) | + +The three ranges quoted as "× faster" are the cases where the *slowest* current-tree run still beat +the *fastest* baseline run, so they hold regardless of which mode each run landed in. The rows marked +"not separable" are changes that remove work but not allocation (a boxed enumerator, a dictionary +hash), and this machine could not resolve them today; they are kept because the mechanism is not in +doubt, not because a number was produced. Worth re-running on a quiet machine. + ## Core reader / writer ### High priority — done @@ -51,30 +84,27 @@ Notes on the two rows that are not a clean win: - [x] **Keep the escape writer vectorized after the first escape** — [JavaScriptUtils.cs](src/Argon/Utilities/JavaScriptUtils.cs). `WriteEscapedJavaScriptNonNullString` used the vectorized `FirstCharToEscape` only to find the first escapable char and then walked the rest of the string one char at a time. It now re-runs that scan on the remaining slice after each escape, so the clean runs between escapes are skipped rather than stepped over. -### Medium priority (per-value allocations) +### Medium priority — done -- [ ] **Span-based Guid probe in `ReadAsBytes`** — `src/Argon/JsonTextReader.cs:97-98`. - `TryConvertGuid(stringReference.ToString(), ...)` allocates a 36-char string for every 36-char byte-string, even when it's base64. Add a `TryConvertGuid(CharSpan)` overload in `ConvertUtils` (`Guid.TryParseExact` has span overloads via Polyfill) and pass `stringReference.AsSpan()`. +- [x] **Span-based Guid probe in `ReadAsBytes`** — [JsonTextReader.cs](src/Argon/JsonTextReader.cs), [ConvertUtils.cs](src/Argon/Utilities/ConvertUtils.cs). + Added a `TryConvertGuid(CharSpan)` overload and pass `stringReference.AsSpan()`, so a 36-char base64 string no longer allocates a string just to be rejected as a Guid. **63% less allocation** on `ReadBase64Strings`. -- [ ] **Stackalloc DateTime write buffers** — `src/Argon/Utilities/DateTimeUtils.cs:136-141, 224-230`. - `WriteDateTimeString` / `WriteDateTimeOffsetString` allocate `new char[64]` per call (hot for date-keyed dictionaries via `JsonSerializerInternalWriter.cs:1051`). Use `stackalloc char[64]` and span-based helpers. +- [x] **Stackalloc DateTime write buffers** — [DateTimeUtils.cs](src/Argon/Utilities/DateTimeUtils.cs). + `WriteDateTimeString` / `WriteDateTimeOffsetString` format into a `stackalloc char[64]`, and the helpers they call (`WriteDefaultIsoDate`, `WriteDateTimeOffset`, `CopyIntToCharArray`) take `Span`. `char[]` callers such as `JsonTextWriter`'s pooled write buffer convert implicitly, so nothing else changed. Paired with the dictionary key item below: **58% less allocation, ~1.6–2.2× faster** on `SerializeDateKeys`. -- [ ] **Fast-path `ToEscapedJavaScriptString` when nothing needs escaping** — `src/Argon/Utilities/JavaScriptUtils.cs:283-300`. - Always builds via `StringWriter` → `StringBuilder` → `ToString()`. When `FirstCharToEscape` returns -1, build the result directly (`string.Create` on modern TFMs, or `string.Concat` with the delimiters). +- [x] **Fast-path `ToEscapedJavaScriptString` when nothing needs escaping** — [JavaScriptUtils.cs](src/Argon/Utilities/JavaScriptUtils.cs). + When the vectorized `FirstCharToEscape` scan comes back -1 the result is copied straight out: the span itself when there are no delimiters, otherwise through a pooled buffer. The `StringWriter` + `StringBuilder` are only built when there is something to escape. **67% less allocation, ~1.4–2.4× faster** on `ToStringNoEscapes`. -- [ ] **Span parameters for `DecimalTryParse` / `Int32TryParse` / `Int64TryParse`** — `src/Argon/JsonReader.cs:760, 786`, `src/Argon/Utilities/ConvertUtils.cs:551, 645, 737`. - `ReadDecimalString` pays `s.ToCharArray()` on every exponent-form decimal (`"96.014e-05"`), a legitimate parse path. The parsers only index their input — change `char[] chars, int start, int length` to `ReadOnlySpan` and pass spans everywhere; no copies. +- [x] **Span parameters for `DecimalTryParse` / `Int32TryParse` / `Int64TryParse`** — [ConvertUtils.cs](src/Argon/Utilities/ConvertUtils.cs), [JsonReader.cs](src/Argon/JsonReader.cs). + The three parsers take `CharSpan` instead of `char[] chars, int start, int length` (the `start`/`length` pair is kept, since `JsonTextReader` passes a slice of its shared buffer). `ReadDecimalString` no longer copies through `ToCharArray()`/`ToArray()` for exponent-form decimals. **42% less allocation** on `ReadExponentDecimals`. -- [ ] **`JsonConvert.ToString(char)` allocates a temp array** — `src/Argon/JsonConvert.cs:92-93`. - `new[]{value}.AsSpan()` heap-allocates per call; use `stackalloc char[1]` or `new ReadOnlySpan(in value)`. +- [x] **`JsonConvert.ToString(char)` allocates a temp array** — [JsonConvert.cs](src/Argon/JsonConvert.cs). + Now a `stackalloc char[1]`. -### Low priority +### Low priority — done -- [ ] **Integer math in `ShiftBufferIfNeeded`** — `src/Argon/JsonTextReader.cs:141`. - `length - charPos <= length * 0.1` does double conversion/multiply once per string/number token; use `(length - charPos) * 10L <= length`. - -- [ ] **(Benchmark first) `ReadNumberIntoBuffer` per-char switch** — `src/Argon/JsonTextReader.cs:1172-1248`. - 28-case switch per digit; `IndexOfAnyExcept` with `SearchValues` of `[0-9a-fA-FxX.+-]` would find the terminator in one call, but numbers are usually short — measure before doing. +- [x] **Integer math in `ShiftBufferIfNeeded`** — [JsonTextReader.cs](src/Argon/JsonTextReader.cs). + `(length - charPos) * 10L <= length`, so the once-per-token check no longer converts to double. The `10L` keeps it correct for buffers past 214M chars. ## Serialization @@ -95,24 +125,24 @@ Notes on the two rows that are not a clean win: - [x] **Single-lookup `SetPropertyPresence`** — [JsonSerializerInternalReader.cs](src/Argon/Serialization/JsonSerializerInternalReader.cs). `ContainsKey` followed by an indexer set hashed the key twice per property. Uses `CollectionsMarshal.GetValueRefOrNullRef` on net6+, with the original two-lookup form kept under `#if` for net4x. -### Medium priority +### Medium priority — done -- [ ] **Cache transformed dictionary keys in naming strategies** — `src/Argon/NamingStrategy/NamingStrategy.cs:46-54` (+ snake/kebab/camel implementations), reached via `DefaultContractResolver.cs:905-913` from `JsonSerializerInternalWriter.cs:991-994`. - With `ProcessDictionaryKeys = true`, every dictionary entry pays a case-conversion allocation per serialization call for keys that repeat across calls. Add a bounded `ThreadSafeStore` cache (key space is user data — cap growth). +- [x] **Cache transformed dictionary keys in naming strategies** — [NamingStrategy.cs](src/Argon/NamingStrategy/NamingStrategy.cs). + `GetDictionaryKey` memoizes resolved keys in a `ConcurrentDictionary` on the strategy instance, created lazily and only when `ProcessDictionaryKeys` is set. Dictionary keys are user data, so the cache stops accepting new entries at 512 — past that keys are still resolved, just not remembered. Caching assumes `ResolvePropertyName` is a pure function of the name, which holds for every strategy in Argon; a strategy that resolves a name from outside state can opt out by overriding the new `CacheDictionaryKeys` to false. **28% less allocation** on `SerializeCamelCaseKeys`. -- [ ] **Kill the StringWriter per DateTime dictionary key** — `src/Argon/Serialization/JsonSerializerInternalWriter.cs:1045-1059`. - `GetDictionaryPropertyName` allocates a `StringWriter` + `StringBuilder` per date key; add direct string-returning overloads in `DateTimeUtils` (pairs with the stackalloc item above). +- [x] **Kill the StringWriter per DateTime dictionary key** — [JsonSerializerInternalWriter.cs](src/Argon/Serialization/JsonSerializerInternalWriter.cs), [DateTimeUtils.cs](src/Argon/Utilities/DateTimeUtils.cs). + `GetDictionaryPropertyName` calls new `ToDateTimeString` / `ToDateTimeOffsetString` overloads that format into a stack buffer and return the string, instead of writing into a `StringWriter` over a `StringBuilder` and calling `ToString` on it. -- [ ] **Cache `$type` name splitting during deserialization** — `src/Argon/Serialization/JsonSerializerInternalReader.cs:646-654`. - `SplitFullyQualifiedTypeName` allocates substrings per `$type` occurrence; only `BindToType` is cached. Cache `string -> TypeNameKey` (bounded — input is untrusted JSON). +- [x] **Cache `$type` name splitting during deserialization** — [JsonSerializerInternalReader.cs](src/Argon/Serialization/JsonSerializerInternalReader.cs). + `SplitTypeName` memoizes `string -> TypeNameKey` on the reader, which is created per deserialize call. Capped at 128 entries since `$type` values come from untrusted JSON; a polymorphic payload repeats a handful of type names, so the cap costs nothing in practice. **33% less allocation** on `DeserializeWithTypeNames`. -### Low priority +### Low priority — done -- [ ] **Indexed loop in `CheckForCircularReference` with custom comparer** — `src/Argon/Serialization/JsonSerializerInternalWriter.cs:269-271`. - `serializeStack.Contains(value, Serializer.EqualityComparer)` is LINQ `Enumerable.Contains` (boxed enumerator per value); replace with a `for` loop. +- [x] **Indexed loop in `CheckForCircularReference` with custom comparer** — [JsonSerializerInternalWriter.cs](src/Argon/Serialization/JsonSerializerInternalWriter.cs). + Extracted `SerializeStackContains`, which walks the list by index when a custom `EqualityComparer` is set rather than going through `Enumerable.Contains` and its boxed enumerator. The no-comparer path still uses `List.Contains`, which was already an indexed scan. -- [ ] **One-time contract creation: duplicate reflection scan + O(n²) `Contains`** — `src/Argon/Serialization/DefaultContractResolver.cs:101-133, 144`. - `GetFieldsAndProperties` runs twice and `defaultMembers.Contains(member)` is linear per member. First-use latency only (result is cached); use a `HashSet` opportunistically. +- [x] **O(n²) `Contains` during one-time contract creation** — [DefaultContractResolver.cs](src/Argon/Serialization/DefaultContractResolver.cs). + `defaultMembers` is a `HashSet`, so the `ShouldSerialize` probe per member is a hash rather than a scan. The other half of that item — the two `GetFieldsAndProperties` calls — was left alone deliberately: the calls pass different `BindingFlags` (public-instance vs public-and-non-public-instance), so collapsing them means re-implementing the binding flag semantics by hand for a first-use-only cost. ## LINQ-to-JSON / JSONPath @@ -130,28 +160,33 @@ Notes on the two rows that are not a clean win: - [x] **Override `GetItem` in `JArray`** — [JArray.cs](src/Argon/Linq/JArray.cs). Indexes the backing list directly instead of going through the virtual `ChildrenTokens` property and an `IList` interface dispatch, mirroring what `IndexOfItem` already did. -### Medium priority +### Medium priority — done -- [ ] **Indexed loops in `ClearItems` / `CopyItemsTo` / `ContentsHashCode`** — `src/Argon/Linq/JContainer.cs:315-327, 361, 614-623`. - `foreach` over interface-typed `children` boxes an enumerator per container; `ContentsHashCode` recurses over whole trees via `JTokenEqualityComparer.GetHashCode`. Use the indexed-loop pattern already used in the copy constructor (lines 26-32). +- [x] **Indexed loops in `ClearItems` / `CopyItemsTo` / `ContentsHashCode`** — [JContainer.cs](src/Argon/Linq/JContainer.cs). + All three index `ChildrenTokens` rather than `foreach`ing over it, matching the copy constructor, so none of them boxes an enumerator any more. -- [ ] **Special-case `JArray`/`JObject` iteration in JSONPath filters** — `src/Argon.JsonPath/ArrayIndexFilter.cs:12-17`, `src/Argon.JsonPath/QueryFilter.cs:8-16`. - `foreach (var v in t)` routes through `Children()` → `JEnumerable` over interface-typed lists (boxed enumerator per input token). `ArrayIndexFilter` already pattern-matches `JArray` — bind it and index the backing list. +- [x] **Special-case `JArray`/`JObject` iteration in JSONPath filters** — [ArrayIndexFilter.cs](src/Argon.JsonPath/ArrayIndexFilter.cs), [QueryFilter.cs](src/Argon.JsonPath/QueryFilter.cs). + `ArrayIndexFilter` binds the `JArray` it already pattern-matches and indexes it. `QueryFilter` walks `First`/`Next` for any `JContainer` — `ChildrenTokens` is `protected`, so it is not reachable from the JsonPath assembly, and the sibling links are the same walk `ScanFilter` uses. Non-containers are skipped, which is what enumerating them produced anyway. One boxed enumerator per input token gone. -- [ ] **Skip triple dictionary hash per property in `JTokenWriter.WritePropertyName`** — `src/Argon/Linq/JTokenWriter.cs:120-131`, `src/Argon/Linq/JObject.cs:104`. - `Remove(name)` + `ValidateToken`'s `Contains(name)` + `AddKey` = three hashes per property on the `FromObject` path. The writer path already flows through `AddAndSkipParentCheck`; let `JObject.InsertItem` skip the duplicate-name `Contains` when that flag is set (the preceding `Remove` guarantees uniqueness). +- [x] **Skip triple dictionary hash per property in `JTokenWriter.WritePropertyName`** — [JContainer.cs](src/Argon/Linq/JContainer.cs), [JObject.cs](src/Argon/Linq/JObject.cs). + `ValidateToken` takes a `skipDuplicateNameCheck` flag, set from `InsertItem`'s `skipParentCheck`. That flag has exactly one source — `AddAndSkipParentCheck`, called only by `JTokenWriter.AddParent`, and both `WritePropertyName` overloads remove any property of that name immediately before — so the duplicate name check is provably redundant there. The type check still runs, and every other path (including `JObject.Load`, which is where a duplicate name in a document is caught) is unchanged. -- [ ] **Iterate `InnerList` in `JObject.CopyTo` (KVP)** — `src/Argon/Linq/JObject.cs:503-510`. - Boxed enumerator + per-item cast; iterate `properties.InnerList` as `GetEnumerator()` does. +- [x] **Iterate `InnerList` in `JObject.CopyTo` (KVP)** — no action needed. + Stale finding: `CopyTo` already iterates `properties.InnerList`, which is typed `List`, so the `foreach` uses the struct enumerator and boxes nothing. ### Low priority -- [ ] **Span-based JSONPath parse for numbers and escape-free strings** — `src/Argon.JsonPath/JPath.cs:563-590, 626-684`. - `TryParseValue` accumulates digits into a `StringBuilder` before parsing (parse the `expression.AsSpan(start, length)` slice instead); `ReadQuotedString` allocates a `StringBuilder` even with no escapes (defer until first `\`, else `Substring`). Mitigated by the path cache, so parse-time only. +- [x] **Span-based JSONPath parse for numbers and escape-free strings** — [JPath.cs](src/Argon.JsonPath/JPath.cs). + `TryParseValue` parses the `expression.AsSpan(start, length)` slice for numbers, and `ReadQuotedString` returns a `Substring` when the string holds no escapes, only building a `StringBuilder` from the first backslash on (copying the run between escapes in one `Append`). The span number parsers need Polyfill, which this project does not reference, so `TryParseInt64`/`TryParseDouble` fall back to a string on net4x. - [ ] **(Awareness only) `JToken.Path` is O(depth × width)** — `src/Argon/Linq/JToken.cs:197-240`. Per array ancestor it does a linear `IndexOf(previous)`; building paths for every element of a big array is quadratic. A real fix needs per-child indices (invasive; matches Newtonsoft behavior as-is). +## Still open + +- [ ] **(Benchmark first) `ReadNumberIntoBuffer` per-char switch** — `src/Argon/JsonTextReader.cs:1172-1248`. + 28-case switch per digit; `IndexOfAnyExcept` with `SearchValues` of `[0-9a-fA-FxX.+-]` would find the terminator in one call, but numbers are usually short. Still open because it cannot be decided without a measurement, and the machine was swinging 2× between runs of identical binaries during this session (see the caveat above), which is far wider than the effect being looked for. `NumberScanBenchmark` in [PerValueBenchmarks.cs](src/ArgonTests/Benchmarks/PerValueBenchmarks.cs) is in place for it: on the current scalar switch it reads 500 short integers in 10.83 µs and 500 long decimals in 43.05 µs. + ## Incidental changes made while implementing the above - **Unblocked the test build** — [AssemblyInfo.cs](src/ArgonTests/AssemblyInfo.cs). @@ -163,7 +198,17 @@ Notes on the two rows that are not a clean win: - **Fixed a pre-existing test failure** — [XmlNodeConverterTest.cs](src/ArgonTests/Converters/XmlNodeConverterTest.cs). `FloatParseHandlingDecimal` failed on both net10.0 and net48, before and after these changes. It built its input as `(decimal) Math.PI + 1000000000m`, but the `double` → `decimal` conversion returns full precision (`3.1415926535897931159979634685`) rather than the 15 significant digits its hardcoded expectation was written for, so it had become a test of conversion precision rather than of the XML/JSON round trip. Confirmed with a standalone console app that no Argon code was involved in producing the differing value. Now uses the decimal literal `1000000003.14159265358979m` directly, so it tests what it intends to; both the XML assertion and the round-trip assertion pass. - Full suite is green: **2358/2358 on net10.0, 2337/2337 on net48, 9/9 F#**. +- **New tests for the JSONPath quoted string parse** — [JPathParseTests.cs](src/ArgonTests/Linq/JsonPath/JPathParseTests.cs). + `SinglePropertyAndFilterWithEscapesAroundText`, `SinglePropertyAndFilterWithEscapeAtEnd` and `SinglePropertyAndFilterWithEmptyString`. Deferring the `StringBuilder` until the first escape means the parser now tracks how much of the expression has been copied in, and the existing tests only covered a single escape (`'h\\i'`) or none at all. These cover text between two escapes, an escape as the final character, and the empty string. + +- **New benchmarks** — [PerValueBenchmarks.cs](src/ArgonTests/Benchmarks/PerValueBenchmarks.cs), registered in [Program.cs](src/Benchmark.Tests/Program.cs). + One per medium/low priority item, in the same shape as `HotPathBenchmarks`. + + Full suite: **2361/2361 net10.0, 2360/2360 net9.0 and net8.0, 2340/2340 net48, 9/9 F#**. net11.0 has + 13 failures, all `double` → `decimal` conversions returning full precision where the expectation was + written for 15 significant digits — the same class of problem as the `FloatParseHandlingDecimal` fix + above, and confirmed to fail identically on a clean worktree at the parent commit, so they are + pre-existing on that preview runtime rather than caused by any of this work. ## Already optimal (checked, no action) From e889459301718149797213532f0fb5363719d8a9 Mon Sep 17 00:00:00 2001 From: Simon Cropp Date: Thu, 27 Aug 2026 20:33:13 +1000 Subject: [PATCH 04/12] Follow the .NET 11 decimal conversion change in the tests .NET 11 makes double and float to decimal conversion correctly rounded instead of truncating to 15 and 7 significant digits (dotnet/runtime#130566, merged for 11.0-preview7, breaking change documented in dotnet/docs#55743). The (decimal) cast was already exact on every runtime; only Convert.ToDecimal changed, and Convert.ToDecimal(Math.PI) now returns 3.1415926535897931159979634685 rather than 3.14159265358979. That left 13 tests failing on net11.0. Argon is unchanged. It uses Convert.ToDecimal for JToken's decimal conversion operator, JValue.Compare and dynamic arithmetic with a decimal operand, and following the platform is right: reintroducing the old truncation would mean deliberately re-adding an inaccuracy the BCL just removed, and would diverge from a (decimal) cast written in the caller's own code. The failures were all in the tests: - Test data that encoded the old truncation now uses decimal literals, so it says what it means rather than depending on the runtime: the serialization event tests built input with Convert.ToDecimal(Math.PI), the DataTable and DataSet tests assigned the double 64.0021 to a decimal typed column, and FloatParseHandling asserted the parsed 1E-06 against Convert.ToDecimal. This is the same fix the runtime team applied to their own two affected tests. - The three copies of the SelectToken documentation sample sum prices read from JSON as doubles, so the total carries the binary expansion of 99.95. They assert on the rounded total. - JValueAddition's decimal assertions compare to 10 decimal places, far more precision than those expressions test. JValueEquals keeps its two genuinely runtime dependent assertions under NET11_0_OR_GREATER: a JValue holding the decimal 1.1 no longer compares equal to the double 1.1. Full suite is green on every framework: 11790 tests, 0 failures. --- .../Converters/DataSetConverterTests.cs | 4 +- .../Converters/DataTableConverterTests.cs | 4 +- .../Documentation/LinqToJsonTests.cs | 4 +- .../JsonPath/QueryJsonSelectTokenWithLinq.cs | 4 +- .../JsonTextReaderTests/FloatTests.cs | 5 +- src/ArgonTests/Linq/DynamicTests.cs | 116 ++++++++++-------- .../Linq/JsonPath/JPathExecuteTests.cs | 4 +- .../Serialization/SerializationEventTests.cs | 7 +- todo.md | 39 +++++- 9 files changed, 125 insertions(+), 62 deletions(-) diff --git a/src/ArgonTests/Converters/DataSetConverterTests.cs b/src/ArgonTests/Converters/DataSetConverterTests.cs index 220d48e72..005f343d1 100644 --- a/src/ArgonTests/Converters/DataSetConverterTests.cs +++ b/src/ArgonTests/Converters/DataSetConverterTests.cs @@ -342,7 +342,9 @@ static DataTable CreateDataTable(string dataTableName, int rows) myNewRow["BooleanCol"] = true; myNewRow["TimeSpanCol"] = new TimeSpan(10, 22, 10, 15, 100); myNewRow["DateTimeCol"] = new DateTime(2000, 12, 29, 0, 0, 0, DateTimeKind.Utc); - myNewRow["DecimalCol"] = 64.0021; + // a decimal literal, not a double: the column is typed decimal, and converting a + // double to decimal is correctly rounded from .NET 11 on rather than truncated + myNewRow["DecimalCol"] = 64.0021m; myTable.Rows.Add(myNewRow); } diff --git a/src/ArgonTests/Converters/DataTableConverterTests.cs b/src/ArgonTests/Converters/DataTableConverterTests.cs index 3063e2467..651eddada 100644 --- a/src/ArgonTests/Converters/DataTableConverterTests.cs +++ b/src/ArgonTests/Converters/DataTableConverterTests.cs @@ -317,7 +317,9 @@ public void Serialize() myNewRow["BooleanCol"] = true; myNewRow["TimeSpanCol"] = new TimeSpan(10, 22, 10, 15, 100); myNewRow["DateTimeCol"] = new DateTime(2000, 12, 29, 0, 0, 0, DateTimeKind.Utc); - myNewRow["DecimalCol"] = 64.0021; + // a decimal literal, not a double: the column is typed decimal, and converting a double + // to decimal is correctly rounded from .NET 11 on rather than truncated to 15 digits + myNewRow["DecimalCol"] = 64.0021m; myNewRow["ArrayCol"] = new[] {1}; myNewRow["BytesCol"] = "Hello world"u8.ToArray(); diff --git a/src/ArgonTests/Documentation/LinqToJsonTests.cs b/src/ArgonTests/Documentation/LinqToJsonTests.cs index 808ee112c..f92cd1d43 100644 --- a/src/ArgonTests/Documentation/LinqToJsonTests.cs +++ b/src/ArgonTests/Documentation/LinqToJsonTests.cs @@ -575,6 +575,8 @@ public void SelectTokenLinq() Assert.Equal(2, storeNames.Count); Assert.Equal(2, firstProductNames.Count); - Assert.Equal(149.95m, totalPrice); + // the prices are read from JSON as doubles, and converting a double to decimal is + // correctly rounded from .NET 11 on, so the sum carries the binary expansion of 99.95 + Assert.Equal(149.95m, Math.Round(totalPrice, 2)); } } \ No newline at end of file diff --git a/src/ArgonTests/Documentation/Samples/JsonPath/QueryJsonSelectTokenWithLinq.cs b/src/ArgonTests/Documentation/Samples/JsonPath/QueryJsonSelectTokenWithLinq.cs index c801a4c02..8618efe59 100644 --- a/src/ArgonTests/Documentation/Samples/JsonPath/QueryJsonSelectTokenWithLinq.cs +++ b/src/ArgonTests/Documentation/Samples/JsonPath/QueryJsonSelectTokenWithLinq.cs @@ -61,6 +61,8 @@ public void Example() #endregion - Assert.Equal(149.95m, totalPrice); + // the prices are read from JSON as doubles, and converting a double to decimal is + // correctly rounded from .NET 11 on, so the sum carries the binary expansion of 99.95 + Assert.Equal(149.95m, Math.Round(totalPrice, 2)); } } \ No newline at end of file diff --git a/src/ArgonTests/JsonTextReaderTests/FloatTests.cs b/src/ArgonTests/JsonTextReaderTests/FloatTests.cs index 4f03d9d5d..097f514c3 100644 --- a/src/ArgonTests/JsonTextReaderTests/FloatTests.cs +++ b/src/ArgonTests/JsonTextReaderTests/FloatTests.cs @@ -206,7 +206,10 @@ public void FloatParseHandling() Assert.Equal(JsonToken.Float, reader.TokenType); Assert.True(reader.Read()); - Assert.Equal(Convert.ToDecimal(1E-06), reader.Value); + // the reader parses the text straight to decimal, so this is exactly 0.000001. It used + // to be written as Convert.ToDecimal(1E-06), which only matched because that conversion + // truncated to 15 significant digits before .NET 11 + Assert.Equal(0.000001m, reader.Value); Assert.Equal(typeof(decimal), reader.ValueType); Assert.Equal(JsonToken.Float, reader.TokenType); diff --git a/src/ArgonTests/Linq/DynamicTests.cs b/src/ArgonTests/Linq/DynamicTests.cs index 17a7d0ad3..13da0b76c 100644 --- a/src/ArgonTests/Linq/DynamicTests.cs +++ b/src/ArgonTests/Linq/DynamicTests.cs @@ -245,7 +245,14 @@ public void JValueEquals() Assert.True(d.Decimal > 0.0f); Assert.True(d.Decimal > null); Assert.True(d.Decimal >= null); + // comparing a decimal against a double converts the double with Convert.ToDecimal, which + // is correctly rounded from .NET 11 on: the double 1.1 converts to 1.100000000000000088817841970 + // rather than to 1.1, so it is no longer equal to the decimal 1.1 +#if NET11_0_OR_GREATER + Assert.True(d.Decimal != 1.1); +#else Assert.True(d.Decimal == 1.1); +#endif Assert.True(d.Decimal == 1.1m); Assert.True(d.Decimal != 1.0f); Assert.True(d.Decimal != 1.0d); @@ -260,7 +267,12 @@ public void JValueEquals() Assert.True(d.Float < 2); Assert.True(d.Float <= 1.1); Assert.True(d.Float == 1.1); + // the same conversion as above, from the other side +#if NET11_0_OR_GREATER + Assert.True(d.Float != 1.1m); +#else Assert.True(d.Float == 1.1m); +#endif Assert.True(d.Float != 1.0f); Assert.True(d.Float != 1.0d); Assert.True(d.Float > new BigInteger(0)); @@ -307,6 +319,10 @@ public void JValueEquals() [Fact] public void JValueAddition() { + // the decimal assertions compare to 10 decimal places: an operand that came through a + // double is converted with Convert.ToDecimal, which is correctly rounded from .NET 11 on + // rather than truncated to 15 significant digits, so those results carry a binary + // expansion far below the precision these expressions are actually testing var o = new JObject( new JProperty("Null", JValue.CreateNull()), new JProperty("Integer", new JValue(1)), @@ -347,9 +363,9 @@ public void JValueAddition() Assert.Equal(4.1, (double) r); r = d.Integer + 1.1d; - Assert.Equal(2.1m, (decimal) r); + Assert.Equal(2.1m, (decimal) r, 10); r += 2; - Assert.Equal(4.1m, (decimal) r); + Assert.Equal(4.1m, (decimal) r, 10); r = d.Integer + null; Assert.Null(r.Value); @@ -367,9 +383,9 @@ public void JValueAddition() Assert.Equal(4.2d, (double) r); r = d.Float + 1.1d; - Assert.Equal(2.2m, (decimal) r); + Assert.Equal(2.2m, (decimal) r, 10); r += 2; - Assert.Equal(4.2m, (decimal) r); + Assert.Equal(4.2m, (decimal) r, 10); r = d.Float + null; Assert.Null(r.Value); @@ -377,19 +393,19 @@ public void JValueAddition() Assert.Null(r.Value); r = d.Decimal + 1; - Assert.Equal(2.1m, (decimal) r); + Assert.Equal(2.1m, (decimal) r, 10); r += 2; - Assert.Equal(4.1m, (decimal) r); + Assert.Equal(4.1m, (decimal) r, 10); r = d.Decimal + 1.1; - Assert.Equal(2.2m, (decimal) r); + Assert.Equal(2.2m, (decimal) r, 10); r += 2; - Assert.Equal(4.2m, (decimal) r); + Assert.Equal(4.2m, (decimal) r, 10); r = d.Decimal + 1.1d; - Assert.Equal(2.2m, (decimal) r); + Assert.Equal(2.2m, (decimal) r, 10); r += 2; - Assert.Equal(4.2m, (decimal) r); + Assert.Equal(4.2m, (decimal) r, 10); r = d.Decimal + null; Assert.Null(r.Value); @@ -407,9 +423,9 @@ public void JValueAddition() Assert.Equal(103, (int) r); r = d.BigInteger + 1.1d; - Assert.Equal(101m, (decimal) r); + Assert.Equal(101m, (decimal) r, 10); r += 2; - Assert.Equal(103m, (decimal) r); + Assert.Equal(103m, (decimal) r, 10); #endregion @@ -426,9 +442,9 @@ public void JValueAddition() Assert.Equal(-2.1d, (double) r); r = d.Integer - 1.1d; - Assert.Equal(-0.1m, (decimal) r); + Assert.Equal(-0.1m, (decimal) r, 10); r -= 2; - Assert.Equal(-2.1m, (decimal) r); + Assert.Equal(-2.1m, (decimal) r, 10); r = d.Integer - null; Assert.Null(r.Value); @@ -446,9 +462,9 @@ public void JValueAddition() Assert.Equal(-2d, (double) r); r = d.Float - 1.1d; - Assert.Equal(0m, (decimal) r); + Assert.Equal(0m, (decimal) r, 10); r -= 2; - Assert.Equal(-2m, (decimal) r); + Assert.Equal(-2m, (decimal) r, 10); r = d.Float - null; Assert.Null(r.Value); @@ -456,19 +472,19 @@ public void JValueAddition() Assert.Null(r.Value); r = d.Decimal - 1; - Assert.Equal(0.1m, (decimal) r); + Assert.Equal(0.1m, (decimal) r, 10); r -= 2; - Assert.Equal(-1.9m, (decimal) r); + Assert.Equal(-1.9m, (decimal) r, 10); r = d.Decimal - 1.1; - Assert.Equal(0m, (decimal) r); + Assert.Equal(0m, (decimal) r, 10); r -= 2; - Assert.Equal(-2m, (decimal) r); + Assert.Equal(-2m, (decimal) r, 10); r = d.Decimal - 1.1d; - Assert.Equal(0m, (decimal) r); + Assert.Equal(0m, (decimal) r, 10); r -= 2; - Assert.Equal(-2m, (decimal) r); + Assert.Equal(-2m, (decimal) r, 10); r = d.Decimal - null; Assert.Null(r.Value); @@ -481,9 +497,9 @@ public void JValueAddition() Assert.Null(r.Value); r = d.BigInteger - 1.1d; - Assert.Equal(99m, (decimal) r); + Assert.Equal(99m, (decimal) r, 10); r -= 2; - Assert.Equal(97m, (decimal) r); + Assert.Equal(97m, (decimal) r, 10); #endregion @@ -500,9 +516,9 @@ public void JValueAddition() Assert.Equal(2.2d, (double) r); r = d.Integer * 1.1d; - Assert.Equal(1.1m, (decimal) r); + Assert.Equal(1.1m, (decimal) r, 10); r *= 2; - Assert.Equal(2.2m, (decimal) r); + Assert.Equal(2.2m, (decimal) r, 10); r = d.Integer * null; Assert.Null(r.Value); @@ -520,9 +536,9 @@ public void JValueAddition() Assert.Equal(2.42d, (double) r, 0.00001); r = d.Float * 1.1d; - Assert.Equal(1.21m, (decimal) r); + Assert.Equal(1.21m, (decimal) r, 10); r *= 2; - Assert.Equal(2.42m, (decimal) r); + Assert.Equal(2.42m, (decimal) r, 10); r = d.Float * null; Assert.Null(r.Value); @@ -530,19 +546,19 @@ public void JValueAddition() Assert.Null(r.Value); r = d.Decimal * 1; - Assert.Equal(1.1m, (decimal) r); + Assert.Equal(1.1m, (decimal) r, 10); r *= 2; - Assert.Equal(2.2m, (decimal) r); + Assert.Equal(2.2m, (decimal) r, 10); r = d.Decimal * 1.1; - Assert.Equal(1.21m, (decimal) r); + Assert.Equal(1.21m, (decimal) r, 10); r *= 2; - Assert.Equal(2.42m, (decimal) r); + Assert.Equal(2.42m, (decimal) r, 10); r = d.Decimal * 1.1d; - Assert.Equal(1.21m, (decimal) r); + Assert.Equal(1.21m, (decimal) r, 10); r *= 2; - Assert.Equal(2.42m, (decimal) r); + Assert.Equal(2.42m, (decimal) r, 10); r = d.Decimal * null; Assert.Null(r.Value); @@ -550,9 +566,9 @@ public void JValueAddition() Assert.Null(r.Value); r = d.BigInteger * 1.1d; - Assert.Equal(100m, (decimal) r); + Assert.Equal(100m, (decimal) r, 10); r *= 2; - Assert.Equal(200m, (decimal) r); + Assert.Equal(200m, (decimal) r, 10); r = d.BigInteger * null; Assert.Null(r.Value); @@ -574,9 +590,9 @@ public void JValueAddition() Assert.Equal(0.454545454545455d, (double) r, 0.00001); r = d.Integer / 1.1d; - Assert.Equal(0.909090909090909m, (decimal) r); + Assert.Equal(0.909090909090909m, (decimal) r, 10); r /= 2; - Assert.Equal(0.454545454545454m, (decimal) r); + Assert.Equal(0.454545454545454m, (decimal) r, 10); r = d.Integer / null; Assert.Null(r.Value); @@ -594,9 +610,9 @@ public void JValueAddition() Assert.Equal(0.5d, (double) r, 0.00001); r = d.Float / 1.1d; - Assert.Equal(1m, (decimal) r); + Assert.Equal(1m, (decimal) r, 10); r /= 2; - Assert.Equal(0.5m, (decimal) r); + Assert.Equal(0.5m, (decimal) r, 10); r = d.Float / null; Assert.Null(r.Value); @@ -604,19 +620,19 @@ public void JValueAddition() Assert.Null(r.Value); r = d.Decimal / 1; - Assert.Equal(1.1m, (decimal) r); + Assert.Equal(1.1m, (decimal) r, 10); r /= 2; - Assert.Equal(0.55m, (decimal) r); + Assert.Equal(0.55m, (decimal) r, 10); r = d.Decimal / 1.1; - Assert.Equal(1m, (decimal) r); + Assert.Equal(1m, (decimal) r, 10); r /= 2; - Assert.Equal(0.5m, (decimal) r); + Assert.Equal(0.5m, (decimal) r, 10); r = d.Decimal / 1.1d; - Assert.Equal(1m, (decimal) r); + Assert.Equal(1m, (decimal) r, 10); r /= 2; - Assert.Equal(0.5m, (decimal) r); + Assert.Equal(0.5m, (decimal) r, 10); r = d.Decimal / null; Assert.Null(r.Value); @@ -624,9 +640,9 @@ public void JValueAddition() Assert.Null(r.Value); r = d.BigInteger / 1.1d; - Assert.Equal(100m, (decimal) r); + Assert.Equal(100m, (decimal) r, 10); r /= 2; - Assert.Equal(50m, (decimal) r); + Assert.Equal(50m, (decimal) r, 10); r = d.BigInteger / null; Assert.Null(r.Value); @@ -716,7 +732,9 @@ public void JValueConvert() AssertValueConverted(99.9m); AssertValueConverted(99.9m); AssertValueConverted(1m); - AssertValueConverted(1.1f, 1.1m); + // Convert.ToDecimal is what Argon uses for this, and it is correctly rounded from .NET 11 + // on: 1.1f converts to 1.10000002384185791015625 there and to 1.1 before that + AssertValueConverted(1.1f, Convert.ToDecimal(1.1f)); AssertValueConverted("1.1", 1.1m); AssertValueConverted(99.9); AssertValueConverted(99.9d); diff --git a/src/ArgonTests/Linq/JsonPath/JPathExecuteTests.cs b/src/ArgonTests/Linq/JsonPath/JPathExecuteTests.cs index e44a2c1fe..15663a19a 100644 --- a/src/ArgonTests/Linq/JsonPath/JPathExecuteTests.cs +++ b/src/ArgonTests/Linq/JsonPath/JPathExecuteTests.cs @@ -1334,7 +1334,9 @@ public void Example() Assert.Equal(2, firstProductNames.Count); Assert.Null(firstProductNames[0]); Assert.Equal("Headlight Fluid", firstProductNames[1]); - Assert.Equal(149.95m, totalPrice); + // the prices are read from JSON as doubles, and converting a double to decimal is + // correctly rounded from .NET 11 on, so the sum carries the binary expansion of 99.95 + Assert.Equal(149.95m, Math.Round(totalPrice, 2)); } [Fact] diff --git a/src/ArgonTests/Serialization/SerializationEventTests.cs b/src/ArgonTests/Serialization/SerializationEventTests.cs index 10a8a748e..bc3bf2ea3 100644 --- a/src/ArgonTests/Serialization/SerializationEventTests.cs +++ b/src/ArgonTests/Serialization/SerializationEventTests.cs @@ -184,7 +184,9 @@ public void ListEvents() 1.1m, 2.222222222m, int.MaxValue, - Convert.ToDecimal(Math.PI) + // the value Convert.ToDecimal(Math.PI) produced before .NET 11 made that conversion + // correctly rounded; written out so the serialized output is the same on every runtime + 3.14159265358979m }; Assert.Equal(11, obj.Member1); @@ -229,7 +231,8 @@ public void DictionaryEvents() {1.1m, "first"}, {2.222222222m, "second"}, {int.MaxValue, "third"}, - {Convert.ToDecimal(Math.PI), "fourth"} + // see ListEvents: a literal keeps the serialized key stable across runtimes + {3.14159265358979m, "fourth"} }; Assert.Equal(11, obj.Member1); diff --git a/todo.md b/todo.md index 6263947ec..841d048ed 100644 --- a/todo.md +++ b/todo.md @@ -204,11 +204,40 @@ doubt, not because a number was produced. Worth re-running on a quiet machine. - **New benchmarks** — [PerValueBenchmarks.cs](src/ArgonTests/Benchmarks/PerValueBenchmarks.cs), registered in [Program.cs](src/Benchmark.Tests/Program.cs). One per medium/low priority item, in the same shape as `HotPathBenchmarks`. - Full suite: **2361/2361 net10.0, 2360/2360 net9.0 and net8.0, 2340/2340 net48, 9/9 F#**. net11.0 has - 13 failures, all `double` → `decimal` conversions returning full precision where the expectation was - written for 15 significant digits — the same class of problem as the `FloatParseHandlingDecimal` fix - above, and confirmed to fail identically on a clean worktree at the parent commit, so they are - pre-existing on that preview runtime rather than caused by any of this work. +- **Fixed the 13 net11.0 failures** — see below. They were pre-existing (they fail identically on a + clean worktree at the parent commit) and are unrelated to the perf work, but they were the only thing + keeping the suite from being green everywhere. + + Full suite: **2361/2361 net10.0, 2360/2360 net11.0, net9.0 and net8.0, 2340/2340 net48, 9/9 F#** — + 11790 tests, 0 failures. + +## The net11.0 decimal failures + +.NET 11 makes `double`/`float` → `decimal` conversion correctly rounded instead of truncating to 15 +(double) or 7 (float) significant digits — [dotnet/runtime#130566](https://github.com/dotnet/runtime/pull/130566), +merged for 11.0-preview7, breaking change documented in dotnet/docs#55743. `Convert.ToDecimal(Math.PI)` +returns `3.1415926535897931159979634685` there and `3.14159265358979` before. The `(decimal)` cast was +already exact on every runtime; only the `Convert` path changed. + +Argon was not changed. It uses `Convert.ToDecimal` for `JToken`'s decimal conversion operator, for +`JValue.Compare`, and for dynamic arithmetic where either operand is a decimal, and following the +platform is the right behaviour — reintroducing the old truncation inside Argon would mean deliberately +re-adding an inaccuracy the BCL just removed, and would diverge from what a `(decimal)` cast in the +caller's own code does. The 13 failures were all in the tests: + +- **Test data that encoded the old truncation** — `SerializationEventTests` built its input with + `Convert.ToDecimal(Math.PI)`, the DataTable/DataSet tests assigned the double `64.0021` to a decimal + typed column, and `FloatTests.FloatParseHandling` asserted against `Convert.ToDecimal(1E-06)`. All now + use decimal literals, so the test says what it means and does not change with the runtime. This is the + same fix the runtime team applied to their own two affected tests in that PR. +- **Documentation samples** — the three copies of the `SelectToken` sample sum prices read from JSON as + doubles, so the total now carries the binary expansion of `99.95`. They assert on the rounded total. +- **Dynamic arithmetic and comparison** — `JValueAddition`'s decimal assertions compare to 10 decimal + places (xUnit's precision overload), which is far more precision than those expressions are testing. + `JValueEquals` has the two genuinely runtime-dependent assertions under `#if NET11_0_OR_GREATER`: a + `JValue` holding the decimal `1.1` no longer compares equal to the double `1.1`, because the double + now converts to `1.100000000000000088817841970`. That is a real, if narrow, behaviour change for + anyone comparing a decimal token against a double on .NET 11. ## Already optimal (checked, no action) From efa007faf00688965c2b4e253d6f6ff4a518432c Mon Sep 17 00:00:00 2001 From: Simon Cropp Date: Thu, 27 Aug 2026 21:57:04 +1000 Subject: [PATCH 05/12] Measure the vectorized number scan and turn it down The last open perf item was to replace the per char switch in ReadNumberIntoBuffer with IndexOfAnyExcept over a SearchValues of the number chars, and it was explicitly marked benchmark first. It was implemented behind a temporary toggle so both scans could be compared in one process, passed the full suite on every framework, and then measured: digits scalar vectorized 1 11.5-12.0 us 14.6 us 25% slower 3 31.4-31.6 us 32.9 us 4% slower 8 44.6-44.9 us 38.4-40.2 us 10% faster 18 71.2-72.1 us 49.9-54.8 us 27% faster Reading 500 numbers, job medium with 3 launches, from the two runs that agreed with each other. The crossover sits between 3 and 8 digits: the vectorized scan has setup cost to earn back and a short number never gives it the chance. Ids, counts and small quantities are most of the numbers in real JSON, so the common case is the one that gets slower. Not taken. A hybrid, scalar for the first 8 chars and vectorized for the rest, also passed the suite but could not be measured to a conclusion: across five runs the same code measured 1.17x to 1.78x apart run to run, wider than the effect. Noted in todo.md as the thing to try first if this is revisited on a quiet machine. The experiment is reverted. NumberScanBenchmark stays, parameterized by digits per number, as the cost profile of number reading by length. --- .../Benchmarks/PerValueBenchmarks.cs | 35 +++++++++--------- todo.md | 36 +++++++++++++++---- 2 files changed, 46 insertions(+), 25 deletions(-) diff --git a/src/ArgonTests/Benchmarks/PerValueBenchmarks.cs b/src/ArgonTests/Benchmarks/PerValueBenchmarks.cs index 260b26650..abb0e3e20 100644 --- a/src/ArgonTests/Benchmarks/PerValueBenchmarks.cs +++ b/src/ArgonTests/Benchmarks/PerValueBenchmarks.cs @@ -187,39 +187,36 @@ public int ReadBase64Strings() } // JsonTextReader.ReadNumberIntoBuffer walks a number a char at a time through a 28 case switch. -// This is here to decide whether finding the terminator with a vectorized IndexOfAnyExcept is -// worth it, given how short numbers usually are. +// Replacing that with a vectorized IndexOfAnyExcept over a SearchValues of the number chars was +// measured and turned down: it costs more than it saves on the short numbers JSON is mostly made +// of. See todo.md. This stays as the cost profile of number reading by length, for whoever picks +// that up next. [MemoryDiagnoser] public class NumberScanBenchmark { - string shortNumbers; - string longNumbers; + string json; + + // digits per number, spanning ids and small quantities through to high precision decimals + [Params(1, 3, 8, 18)] + public int Digits { get; set; } [GlobalSetup] public void Setup() { - // the length JSON numbers usually are: ids, counts, small quantities - shortNumbers = $"[{string.Join(",", Enumerable.Range(0, 500).Select(_ => _ % 1000))}]"; - - // long enough for a vectorized scan to have something to do - longNumbers = $"[{string.Join(",", Enumerable.Range(0, 500).Select(_ => $"{_}.123456789012345"))}]"; + var number = Digits <= 2 + ? new string('7', Digits) + : $"{new string('7', Digits - 2)}.7"; + json = $"[{string.Join(",", Enumerable.Repeat(number, 500))}]"; } [Benchmark] - public long ReadShortNumbers() => - Sum(shortNumbers); - - [Benchmark] - public long ReadLongNumbers() => - Sum(longNumbers); - - static long Sum(string json) + public int ReadNumbers() { using var reader = new JsonTextReader(new StringReader(json)); - long count = 0; + var count = 0; while (reader.Read()) { - if (reader.TokenType == JsonToken.Integer || reader.TokenType == JsonToken.Float) + if (reader.TokenType is JsonToken.Integer or JsonToken.Float) { count++; } diff --git a/todo.md b/todo.md index 841d048ed..38211086e 100644 --- a/todo.md +++ b/todo.md @@ -3,8 +3,8 @@ Findings from a perf review of the core read/write path, serialization, and LINQ-to-JSON / JSONPath (2026-08-27). Prioritized within each section; high-priority items sit on per-character / per-token / per-property hot paths. -**All high, medium and low priority items are implemented**, except one that was explicitly -"benchmark first" and could not be measured reliably — see [Still open](#still-open). +**All high, medium and low priority items are resolved.** One was explicitly "benchmark first"; +it was measured and turned down — see [Measured and turned down](#measured-and-turned-down). ## Measured impact — high priority @@ -182,10 +182,34 @@ doubt, not because a number was produced. Worth re-running on a quiet machine. - [ ] **(Awareness only) `JToken.Path` is O(depth × width)** — `src/Argon/Linq/JToken.cs:197-240`. Per array ancestor it does a linear `IndexOf(previous)`; building paths for every element of a big array is quadratic. A real fix needs per-child indices (invasive; matches Newtonsoft behavior as-is). -## Still open - -- [ ] **(Benchmark first) `ReadNumberIntoBuffer` per-char switch** — `src/Argon/JsonTextReader.cs:1172-1248`. - 28-case switch per digit; `IndexOfAnyExcept` with `SearchValues` of `[0-9a-fA-FxX.+-]` would find the terminator in one call, but numbers are usually short. Still open because it cannot be decided without a measurement, and the machine was swinging 2× between runs of identical binaries during this session (see the caveat above), which is far wider than the effect being looked for. `NumberScanBenchmark` in [PerValueBenchmarks.cs](src/ArgonTests/Benchmarks/PerValueBenchmarks.cs) is in place for it: on the current scalar switch it reads 500 short integers in 10.83 µs and 500 long decimals in 43.05 µs. +## Measured and turned down + +- [x] **(Benchmark first) `ReadNumberIntoBuffer` per-char switch** — measured, not taken. + The idea was to replace the 28-case switch per digit with `IndexOfAnyExcept` over a + `SearchValues` of `[0-9a-fA-FxX.+-]`, finding the terminator in one call. It was implemented + behind a temporary toggle so both scans could be compared in the same process, and it passed the + full suite on every framework, so the implementation was sound. It is not worth taking: + + | Digits per number | Scalar switch | Vectorized | | + |---|---|---|---| + | 1 | 11.5–12.0 µs | 14.6 µs | **25% slower** | + | 3 | 31.4–31.6 µs | 32.9 µs | **4% slower** | + | 8 | 44.6–44.9 µs | 38.4–40.2 µs | 10% faster | + | 18 | 71.2–72.1 µs | 49.9–54.8 µs | 27% faster | + + Reading 500 numbers, `--job medium --launchCount 3`, from the two runs that agreed with each + other. The crossover sits between 3 and 8 digits: the vectorized scan has setup cost to earn back, + and a short number never gives it the chance. The premise in the original finding — that numbers + are usually short — is what decides it, since ids, counts and small quantities are most of the + numbers in real JSON, and those get slower. + + A hybrid (scalar for the first 8 chars, vectorized for whatever is left) should in principle take + the win without the loss, and also passed the full suite, but it could not be measured to a + conclusion: across five runs the *same* code measured 1.17× to 1.78× apart run to run, which is + wider than the effect. That is the thing to try first if this is ever revisited on a quiet + machine. The experiment itself is reverted; `NumberScanBenchmark` in + [PerValueBenchmarks.cs](src/ArgonTests/Benchmarks/PerValueBenchmarks.cs) stays as the cost profile + of number reading by length. ## Incidental changes made while implementing the above From 8ff0362af0d905afcbbab43a27c3cff401cb14eb Mon Sep 17 00:00:00 2001 From: Simon Cropp Date: Thu, 27 Aug 2026 22:53:28 +1000 Subject: [PATCH 06/12] Delete todo.md --- todo.md | 271 -------------------------------------------------------- 1 file changed, 271 deletions(-) delete mode 100644 todo.md diff --git a/todo.md b/todo.md deleted file mode 100644 index 38211086e..000000000 --- a/todo.md +++ /dev/null @@ -1,271 +0,0 @@ -# Performance improvement todo - -Findings from a perf review of the core read/write path, serialization, and LINQ-to-JSON / JSONPath (2026-08-27). -Prioritized within each section; high-priority items sit on per-character / per-token / per-property hot paths. - -**All high, medium and low priority items are resolved.** One was explicitly "benchmark first"; -it was measured and turned down — see [Measured and turned down](#measured-and-turned-down). - -## Measured impact — high priority - -BenchmarkDotNet `--job short`, .NET 10.0.11, AMD Ryzen 9 5900X. Baseline is a clean worktree at the -commit these changes sit on, running the identical benchmark code. Benchmarks live in -[HotPathBenchmarks.cs](src/ArgonTests/Benchmarks/HotPathBenchmarks.cs), plus the pre-existing -`ReaderBenchmarks`, `WriterBenchmarks` and `JsonPathRegexBenchmark`. - -| Benchmark | Before | After | Time | Allocation | -|---|---|---|---|---| -| `ReadStringHeavy` | 5.727 µs | 1.611 µs | **3.6× faster** | unchanged | -| `SerializeWithTypeNames` | 121.2 µs / 265 KB | 36.21 µs / 89.9 KB | **3.3× faster** | **−66%** | -| `SerializeWithConverters` | 70.65 µs | 32.56 µs | **2.2× faster** | +0.5 KB (memo) | -| `GetInternedNames` | 2.081 µs | 1.083 µs | **1.9× faster** | none either way | -| `DeserializeWithConverters` | 86.20 µs | 47.20 µs | **1.8× faster** | +0.5 KB (memo) | -| `WriteSpanPropertyNames` | 1.208 µs / 3.46 KB | 693.5 ns / 2.51 KB | **1.7× faster** | **−27%** | -| `DeserializeWideRecord` | 2.544 µs | 1.628 µs | **1.6× faster** | unchanged | -| `EnumerateProperties` | 301.8 ns / 88 B | 227.2 ns / 64 B | **1.3× faster** | **−27%** | -| `PopulateExistingValues` | 1.727 µs | 1.317 µs | **1.3× faster** | +0.05 KB | -| `IndexArray` | 2.107 µs | 1.998 µs | 1.05× faster | none either way | -| `IntArrayFromObject` | 27.5 µs / 69.1 KB | 29.6 µs / 57.1 KB | within noise | **−17%** | - -Notes on the two rows that are not a clean win: - -- The converter memo adds ~0.5 KB per serialize/deserialize call for the `Dictionary` - itself. That buys roughly a halving of wall-clock whenever converters are registered, and the - dictionary is never allocated at all when the converter list is empty. -- `IntArrayFromObject` time moved within the measurement error of both runs (the confidence intervals - overlap); the 17% allocation drop from routing `WriteValue(int)` through `BoxedPrimitives` is the real - and repeatable part. - -## Measured impact — medium and low priority - -Benchmarks in [PerValueBenchmarks.cs](src/ArgonTests/Benchmarks/PerValueBenchmarks.cs), same baseline -worktree and machine, `--job medium`. - -**Read the timings with the caveat below.** During this measurement session the machine alternated -between two performance modes roughly 2× apart: the *same* baseline binary measured 54.5 µs and -90.6 µs for `DeserializeWithTypeNames` in back-to-back runs, and a current-tree run measured 2.15 µs -then 1.00 µs for `FromObjectWide`. Runs were therefore ordered ABBA (current, baseline, baseline, -current) to expose the drift, and only differences that hold *within* a mode are reported as wins. -Allocation numbers are deterministic and are unaffected by any of this. - -| Benchmark | Allocated before → after | Time | -|---|---|---| -| `SerializeDateKeys` | 60.30 KB → 25.15 KB (**−58%**) | 14.75 µs → 6.66 µs (**~1.6–2.2× faster**) | -| `SerializeDateTimeOffsetKeys` | 62.85 KB → 27.70 KB (**−56%**) | 15.94 µs → 7.79 µs (**~1.4–2.0× faster**) | -| `ToStringNoEscapes` | 35.08 KB → 11.72 KB (**−67%**) | 4.31 µs → 1.79 µs (**~1.4–2.4× faster**) | -| `ReadBase64Strings` | 29.92 KB → 11.17 KB (**−63%**) | 10.68 µs → 7.94 µs (best of each; modes did not line up) | -| `ReadExponentDecimals` | 22.11 KB → 12.73 KB (**−42%**) | within noise | -| `DeserializeWithTypeNames` | 83.02 KB → 55.28 KB (**−33%**) | ~4% faster in both modes (90.6→87.0, 54.5→52.0) | -| `SerializeCamelCaseKeys` | 22.59 KB → 16.34 KB (**−28%**) | within noise in the one mode measured on both | -| `WildcardIndexFilter` | 448 B → 416 B | ~1% in mode; one boxed enumerator per input token gone | -| `QueryFilter` | 29176 B → 29136 B | not separable from noise | -| `ParseAndQuery` | 43712 B → 43672 B | not separable from noise | -| `FromObjectWide` | unchanged (4.84 KB) | not separable from noise (one of three name hashes per property gone) | - -The three ranges quoted as "× faster" are the cases where the *slowest* current-tree run still beat -the *fastest* baseline run, so they hold regardless of which mode each run landed in. The rows marked -"not separable" are changes that remove work but not allocation (a boxed enumerator, a dictionary -hash), and this machine could not resolve them today; they are kept because the mechanism is not in -doubt, not because a number was produced. Worth re-running on a quiet machine. - -## Core reader / writer - -### High priority — done - -- [x] **Vectorize `ReadStringIntoBuffer`** — [JsonTextReader.cs](src/Argon/JsonTextReader.cs). - The hottest loop in the reader scanned every string char-by-char through a scalar `switch`, but typical strings contain none of the six interesting chars (`\0 \\ \r \n " '`). Added `SkipToNextStringDelimiter`, which uses a static `SearchValues` to jump straight to the next char the switch actually acts on; the `'\0'` terminator kept at `charsUsed` doubles as the scan's stop sentinel, so the "need more data" path is unchanged. Guarded to net8+, with the original scalar walk on older TFMs. **3.6× faster** on `ReadStringHeavy`. - -- [x] **Stop allocating a string per property in span-based `WritePropertyName`** — [JsonWriter.cs](src/Argon/JsonWriter.cs), [JsonPosition.cs](src/Argon/JsonPosition.cs). - `InternalWritePropertyName(name.ToString())` materialized the span on every call, so the span overload allocated exactly as much as the string one. `JsonPosition` now also holds `NameChars`/`NameLength`, and the span overload copies into a buffer owned by the position and reused by every property at that depth. Path building reads whichever of the two is set. **1.7× faster, 27% less allocation.** - -- [x] **Vectorize `DefaultJsonNameTable.TextEquals`** — [DefaultJsonNameTable.cs](src/Argon/DefaultJsonNameTable.cs). - Replaced the manual char loop with `str1.AsSpan().SequenceEqual(str2.AsSpan(str2Start, str2Length))`, which also short circuits the length mismatch. **1.9× faster** on `GetInternedNames`. - -- [x] **Keep the escape writer vectorized after the first escape** — [JavaScriptUtils.cs](src/Argon/Utilities/JavaScriptUtils.cs). - `WriteEscapedJavaScriptNonNullString` used the vectorized `FirstCharToEscape` only to find the first escapable char and then walked the rest of the string one char at a time. It now re-runs that scan on the remaining slice after each escape, so the clean runs between escapes are skipped rather than stepped over. - -### Medium priority — done - -- [x] **Span-based Guid probe in `ReadAsBytes`** — [JsonTextReader.cs](src/Argon/JsonTextReader.cs), [ConvertUtils.cs](src/Argon/Utilities/ConvertUtils.cs). - Added a `TryConvertGuid(CharSpan)` overload and pass `stringReference.AsSpan()`, so a 36-char base64 string no longer allocates a string just to be rejected as a Guid. **63% less allocation** on `ReadBase64Strings`. - -- [x] **Stackalloc DateTime write buffers** — [DateTimeUtils.cs](src/Argon/Utilities/DateTimeUtils.cs). - `WriteDateTimeString` / `WriteDateTimeOffsetString` format into a `stackalloc char[64]`, and the helpers they call (`WriteDefaultIsoDate`, `WriteDateTimeOffset`, `CopyIntToCharArray`) take `Span`. `char[]` callers such as `JsonTextWriter`'s pooled write buffer convert implicitly, so nothing else changed. Paired with the dictionary key item below: **58% less allocation, ~1.6–2.2× faster** on `SerializeDateKeys`. - -- [x] **Fast-path `ToEscapedJavaScriptString` when nothing needs escaping** — [JavaScriptUtils.cs](src/Argon/Utilities/JavaScriptUtils.cs). - When the vectorized `FirstCharToEscape` scan comes back -1 the result is copied straight out: the span itself when there are no delimiters, otherwise through a pooled buffer. The `StringWriter` + `StringBuilder` are only built when there is something to escape. **67% less allocation, ~1.4–2.4× faster** on `ToStringNoEscapes`. - -- [x] **Span parameters for `DecimalTryParse` / `Int32TryParse` / `Int64TryParse`** — [ConvertUtils.cs](src/Argon/Utilities/ConvertUtils.cs), [JsonReader.cs](src/Argon/JsonReader.cs). - The three parsers take `CharSpan` instead of `char[] chars, int start, int length` (the `start`/`length` pair is kept, since `JsonTextReader` passes a slice of its shared buffer). `ReadDecimalString` no longer copies through `ToCharArray()`/`ToArray()` for exponent-form decimals. **42% less allocation** on `ReadExponentDecimals`. - -- [x] **`JsonConvert.ToString(char)` allocates a temp array** — [JsonConvert.cs](src/Argon/JsonConvert.cs). - Now a `stackalloc char[1]`. - -### Low priority — done - -- [x] **Integer math in `ShiftBufferIfNeeded`** — [JsonTextReader.cs](src/Argon/JsonTextReader.cs). - `(length - charPos) * 10L <= length`, so the once-per-token check no longer converts to double. The `10L` keeps it correct for buffers past 214M chars. - -## Serialization - -### High priority — done - -- [x] **Memoize the per-value converter scan** — [JsonSerializerInternalBase.cs](src/Argon/Serialization/JsonSerializerInternalBase.cs), used from [JsonSerializerInternalWriter.cs](src/Argon/Serialization/JsonSerializerInternalWriter.cs) and [JsonSerializerInternalReader.cs](src/Argon/Serialization/JsonSerializerInternalReader.cs). - `GetMatchingConverter` walked the converter list calling virtual `CanConvert(type)` for every value serialized and every property, item and dictionary entry deserialized. Now memoized in a `Dictionary` on the internal base, which is instantiated fresh per serialize/deserialize call, so converters registered between calls are still picked up. Not cacheable on the contract, since contracts are shared across serializers with different converter lists. **2.2× faster serializing, 1.8× deserializing** with converters registered. - -- [x] **Cache formatted `$type` names** — [JsonSerializerInternalWriter.cs](src/Argon/Serialization/JsonSerializerInternalWriter.cs). - Every `$type` concatenated the type and assembly names and re-parsed the result through `RemoveAssemblyDetails`, allocating a `StringBuilder` and a string per object. Now cached per run, keyed on type alone — the binder and format handling are fixed for the duration of a serialization. **3.3× faster, 66% less allocation.** - -- [x] **Drop duplicate contract resolution in `CalculatePropertyDetails`** — [JsonSerializerInternalReader.cs](src/Argon/Serialization/JsonSerializerInternalReader.cs). - `GetContract(currentValue.GetType())` ran twice with the same argument per populated property. `currentValue` is only ever non-null when the earlier block ran, and that block already resolved the same contract, so the second resolution is gone. **1.3× faster** on `PopulateExistingValues`. - -- [x] **Fix O(n²) creator-parameter index lookup** — [JsonObjectContract.cs](src/Argon/Serialization/JsonObjectContract.cs). - `CreatorParameters.IndexOf(constructorProperty)` was a linear scan per matched parameter, making every object built through a parameterized constructor quadratic in its parameter count. Added `IndexOfCreatorParameter`, backed by a `Dictionary` built on first use (the resolver populates `CreatorParameters` after construction) and rebuilt if that collection later changes. **1.6× faster** deserializing a 16-parameter record. - -- [x] **Single-lookup `SetPropertyPresence`** — [JsonSerializerInternalReader.cs](src/Argon/Serialization/JsonSerializerInternalReader.cs). - `ContainsKey` followed by an indexer set hashed the key twice per property. Uses `CollectionsMarshal.GetValueRefOrNullRef` on net6+, with the original two-lookup form kept under `#if` for net4x. - -### Medium priority — done - -- [x] **Cache transformed dictionary keys in naming strategies** — [NamingStrategy.cs](src/Argon/NamingStrategy/NamingStrategy.cs). - `GetDictionaryKey` memoizes resolved keys in a `ConcurrentDictionary` on the strategy instance, created lazily and only when `ProcessDictionaryKeys` is set. Dictionary keys are user data, so the cache stops accepting new entries at 512 — past that keys are still resolved, just not remembered. Caching assumes `ResolvePropertyName` is a pure function of the name, which holds for every strategy in Argon; a strategy that resolves a name from outside state can opt out by overriding the new `CacheDictionaryKeys` to false. **28% less allocation** on `SerializeCamelCaseKeys`. - -- [x] **Kill the StringWriter per DateTime dictionary key** — [JsonSerializerInternalWriter.cs](src/Argon/Serialization/JsonSerializerInternalWriter.cs), [DateTimeUtils.cs](src/Argon/Utilities/DateTimeUtils.cs). - `GetDictionaryPropertyName` calls new `ToDateTimeString` / `ToDateTimeOffsetString` overloads that format into a stack buffer and return the string, instead of writing into a `StringWriter` over a `StringBuilder` and calling `ToString` on it. - -- [x] **Cache `$type` name splitting during deserialization** — [JsonSerializerInternalReader.cs](src/Argon/Serialization/JsonSerializerInternalReader.cs). - `SplitTypeName` memoizes `string -> TypeNameKey` on the reader, which is created per deserialize call. Capped at 128 entries since `$type` values come from untrusted JSON; a polymorphic payload repeats a handful of type names, so the cap costs nothing in practice. **33% less allocation** on `DeserializeWithTypeNames`. - -### Low priority — done - -- [x] **Indexed loop in `CheckForCircularReference` with custom comparer** — [JsonSerializerInternalWriter.cs](src/Argon/Serialization/JsonSerializerInternalWriter.cs). - Extracted `SerializeStackContains`, which walks the list by index when a custom `EqualityComparer` is set rather than going through `Enumerable.Contains` and its boxed enumerator. The no-comparer path still uses `List.Contains`, which was already an indexed scan. - -- [x] **O(n²) `Contains` during one-time contract creation** — [DefaultContractResolver.cs](src/Argon/Serialization/DefaultContractResolver.cs). - `defaultMembers` is a `HashSet`, so the `ShouldSerialize` probe per member is a hash rather than a scan. The other half of that item — the two `GetFieldsAndProperties` calls — was left alone deliberately: the calls pass different `BindingFlags` (public-instance vs public-and-non-public-instance), so collapsing them means re-implementing the binding flag semantics by hand for a first-use-only cost. - -## LINQ-to-JSON / JSONPath - -### High priority — done - -- [x] **Replace LINQ `Cast()` in `JObject.Properties()`** — [JObject.cs](src/Argon/Linq/JObject.cs). - Now an iterator over `properties.InnerList` (the pattern `GetEnumerator()` already used), so there is no LINQ wrapper enumerable and no boxed interface enumerator. `CopyTo` got the same treatment. **1.3× faster, 27% less allocation.** - -- [x] **Route `JTokenWriter.WriteValue(int)` through `BoxedPrimitives`** — [JTokenWriter.cs](src/Argon/Linq/JTokenWriter.cs). - `int` is the most common CLR type for a JSON number and was the only numeric overload still boxing at the call site. Deliberately not applied to `short`/`ushort`/`byte`/`sbyte`/`uint`: those widen to `int`, which would change the CLR type stored in `JValue.Value` and so is a behaviour change, not just a perf one. **17% less allocation** on `IntArrayFromObject`. - -- [x] **Store a `Regex` instance in JSONPath `=~` expressions** — [BooleanQueryExpression.cs](src/Argon.JsonPath/BooleanQueryExpression.cs). - Evaluation went through static `Regex.IsMatch` per candidate token — a process-wide cache probe each time, degrading to a full pattern re-parse once more than `Regex.CacheSize` patterns are in play. The constructed `Regex` is now cached with the timeout it was built for, in a single reference field so a concurrently evaluated cached `JPath` cannot observe a regex paired with the wrong timeout. Built lazily so an invalid pattern still surfaces during evaluation rather than at parse time. - -- [x] **Override `GetItem` in `JArray`** — [JArray.cs](src/Argon/Linq/JArray.cs). - Indexes the backing list directly instead of going through the virtual `ChildrenTokens` property and an `IList` interface dispatch, mirroring what `IndexOfItem` already did. - -### Medium priority — done - -- [x] **Indexed loops in `ClearItems` / `CopyItemsTo` / `ContentsHashCode`** — [JContainer.cs](src/Argon/Linq/JContainer.cs). - All three index `ChildrenTokens` rather than `foreach`ing over it, matching the copy constructor, so none of them boxes an enumerator any more. - -- [x] **Special-case `JArray`/`JObject` iteration in JSONPath filters** — [ArrayIndexFilter.cs](src/Argon.JsonPath/ArrayIndexFilter.cs), [QueryFilter.cs](src/Argon.JsonPath/QueryFilter.cs). - `ArrayIndexFilter` binds the `JArray` it already pattern-matches and indexes it. `QueryFilter` walks `First`/`Next` for any `JContainer` — `ChildrenTokens` is `protected`, so it is not reachable from the JsonPath assembly, and the sibling links are the same walk `ScanFilter` uses. Non-containers are skipped, which is what enumerating them produced anyway. One boxed enumerator per input token gone. - -- [x] **Skip triple dictionary hash per property in `JTokenWriter.WritePropertyName`** — [JContainer.cs](src/Argon/Linq/JContainer.cs), [JObject.cs](src/Argon/Linq/JObject.cs). - `ValidateToken` takes a `skipDuplicateNameCheck` flag, set from `InsertItem`'s `skipParentCheck`. That flag has exactly one source — `AddAndSkipParentCheck`, called only by `JTokenWriter.AddParent`, and both `WritePropertyName` overloads remove any property of that name immediately before — so the duplicate name check is provably redundant there. The type check still runs, and every other path (including `JObject.Load`, which is where a duplicate name in a document is caught) is unchanged. - -- [x] **Iterate `InnerList` in `JObject.CopyTo` (KVP)** — no action needed. - Stale finding: `CopyTo` already iterates `properties.InnerList`, which is typed `List`, so the `foreach` uses the struct enumerator and boxes nothing. - -### Low priority - -- [x] **Span-based JSONPath parse for numbers and escape-free strings** — [JPath.cs](src/Argon.JsonPath/JPath.cs). - `TryParseValue` parses the `expression.AsSpan(start, length)` slice for numbers, and `ReadQuotedString` returns a `Substring` when the string holds no escapes, only building a `StringBuilder` from the first backslash on (copying the run between escapes in one `Append`). The span number parsers need Polyfill, which this project does not reference, so `TryParseInt64`/`TryParseDouble` fall back to a string on net4x. - -- [ ] **(Awareness only) `JToken.Path` is O(depth × width)** — `src/Argon/Linq/JToken.cs:197-240`. - Per array ancestor it does a linear `IndexOf(previous)`; building paths for every element of a big array is quadratic. A real fix needs per-child indices (invasive; matches Newtonsoft behavior as-is). - -## Measured and turned down - -- [x] **(Benchmark first) `ReadNumberIntoBuffer` per-char switch** — measured, not taken. - The idea was to replace the 28-case switch per digit with `IndexOfAnyExcept` over a - `SearchValues` of `[0-9a-fA-FxX.+-]`, finding the terminator in one call. It was implemented - behind a temporary toggle so both scans could be compared in the same process, and it passed the - full suite on every framework, so the implementation was sound. It is not worth taking: - - | Digits per number | Scalar switch | Vectorized | | - |---|---|---|---| - | 1 | 11.5–12.0 µs | 14.6 µs | **25% slower** | - | 3 | 31.4–31.6 µs | 32.9 µs | **4% slower** | - | 8 | 44.6–44.9 µs | 38.4–40.2 µs | 10% faster | - | 18 | 71.2–72.1 µs | 49.9–54.8 µs | 27% faster | - - Reading 500 numbers, `--job medium --launchCount 3`, from the two runs that agreed with each - other. The crossover sits between 3 and 8 digits: the vectorized scan has setup cost to earn back, - and a short number never gives it the chance. The premise in the original finding — that numbers - are usually short — is what decides it, since ids, counts and small quantities are most of the - numbers in real JSON, and those get slower. - - A hybrid (scalar for the first 8 chars, vectorized for whatever is left) should in principle take - the win without the loss, and also passed the full suite, but it could not be measured to a - conclusion: across five runs the *same* code measured 1.17× to 1.78× apart run to run, which is - wider than the effect. That is the thing to try first if this is ever revisited on a quiet - machine. The experiment itself is reverted; `NumberScanBenchmark` in - [PerValueBenchmarks.cs](src/ArgonTests/Benchmarks/PerValueBenchmarks.cs) stays as the cost profile - of number reading by length. - -## Incidental changes made while implementing the above - -- **Unblocked the test build** — [AssemblyInfo.cs](src/ArgonTests/AssemblyInfo.cs). - `[assembly: CollectionBehavior(DisableTestParallelization = true)]` became a build error (obsolete as error) after the xUnit v3 4.0.0 bump, so no tests could run at all. Replaced with the current API, `[assembly: Xunit.v3.Parallelization(Mode = Xunit.Sdk.ParallelMode.None)]`. - -- **New tests for the deferred property name** — [JsonTextWriterTest.cs](src/ArgonTests/JsonTextWriterTest.cs). - `PathWithSpanPropertyNames` and `SpanPropertyNameInExceptionPath`. The span `WritePropertyName` change is the only one that alters how a value is stored and read back later, and there was no existing coverage pairing the span overload with `Path`. They cover names changing length at one depth, names surviving a push/pop, names needing path escaping, a span sliced out of a larger buffer, and mixing the string and span overloads. - -- **Fixed a pre-existing test failure** — [XmlNodeConverterTest.cs](src/ArgonTests/Converters/XmlNodeConverterTest.cs). - `FloatParseHandlingDecimal` failed on both net10.0 and net48, before and after these changes. It built its input as `(decimal) Math.PI + 1000000000m`, but the `double` → `decimal` conversion returns full precision (`3.1415926535897931159979634685`) rather than the 15 significant digits its hardcoded expectation was written for, so it had become a test of conversion precision rather than of the XML/JSON round trip. Confirmed with a standalone console app that no Argon code was involved in producing the differing value. Now uses the decimal literal `1000000003.14159265358979m` directly, so it tests what it intends to; both the XML assertion and the round-trip assertion pass. - -- **New tests for the JSONPath quoted string parse** — [JPathParseTests.cs](src/ArgonTests/Linq/JsonPath/JPathParseTests.cs). - `SinglePropertyAndFilterWithEscapesAroundText`, `SinglePropertyAndFilterWithEscapeAtEnd` and `SinglePropertyAndFilterWithEmptyString`. Deferring the `StringBuilder` until the first escape means the parser now tracks how much of the expression has been copied in, and the existing tests only covered a single escape (`'h\\i'`) or none at all. These cover text between two escapes, an escape as the final character, and the empty string. - -- **New benchmarks** — [PerValueBenchmarks.cs](src/ArgonTests/Benchmarks/PerValueBenchmarks.cs), registered in [Program.cs](src/Benchmark.Tests/Program.cs). - One per medium/low priority item, in the same shape as `HotPathBenchmarks`. - -- **Fixed the 13 net11.0 failures** — see below. They were pre-existing (they fail identically on a - clean worktree at the parent commit) and are unrelated to the perf work, but they were the only thing - keeping the suite from being green everywhere. - - Full suite: **2361/2361 net10.0, 2360/2360 net11.0, net9.0 and net8.0, 2340/2340 net48, 9/9 F#** — - 11790 tests, 0 failures. - -## The net11.0 decimal failures - -.NET 11 makes `double`/`float` → `decimal` conversion correctly rounded instead of truncating to 15 -(double) or 7 (float) significant digits — [dotnet/runtime#130566](https://github.com/dotnet/runtime/pull/130566), -merged for 11.0-preview7, breaking change documented in dotnet/docs#55743. `Convert.ToDecimal(Math.PI)` -returns `3.1415926535897931159979634685` there and `3.14159265358979` before. The `(decimal)` cast was -already exact on every runtime; only the `Convert` path changed. - -Argon was not changed. It uses `Convert.ToDecimal` for `JToken`'s decimal conversion operator, for -`JValue.Compare`, and for dynamic arithmetic where either operand is a decimal, and following the -platform is the right behaviour — reintroducing the old truncation inside Argon would mean deliberately -re-adding an inaccuracy the BCL just removed, and would diverge from what a `(decimal)` cast in the -caller's own code does. The 13 failures were all in the tests: - -- **Test data that encoded the old truncation** — `SerializationEventTests` built its input with - `Convert.ToDecimal(Math.PI)`, the DataTable/DataSet tests assigned the double `64.0021` to a decimal - typed column, and `FloatTests.FloatParseHandling` asserted against `Convert.ToDecimal(1E-06)`. All now - use decimal literals, so the test says what it means and does not change with the runtime. This is the - same fix the runtime team applied to their own two affected tests in that PR. -- **Documentation samples** — the three copies of the `SelectToken` sample sum prices read from JSON as - doubles, so the total now carries the binary expansion of `99.95`. They assert on the rounded total. -- **Dynamic arithmetic and comparison** — `JValueAddition`'s decimal assertions compare to 10 decimal - places (xUnit's precision overload), which is far more precision than those expressions are testing. - `JValueEquals` has the two genuinely runtime-dependent assertions under `#if NET11_0_OR_GREATER`: a - `JValue` holding the decimal `1.1` no longer compares equal to the double `1.1`, because the double - now converts to `1.100000000000000088817841970`. That is a real, if narrow, behaviour change for - anyone comparing a decimal token against a double on .NET 11. - -## Already optimal (checked, no action) - -- Write-side escape scanning uses `SearchValues` with a ≥16-char threshold; `JsonTextWriter` uses `TryFormat` into pooled buffers; `BoxedPrimitives` covers `JValue(long/bool/double/decimal)`. -- `StringBuffer`/`BufferUtils`/reader `charBuffer` are ArrayPool-backed; `Base64Encoder` has a stackalloc net6+ path; `ConvertUtils.GetTypeCode` uses `FrozenDictionary`. -- Contract property names are interned via `DefaultJsonNameTable`; presence dictionaries have capacity hints; `EnumUtils` caches per (enum, naming strategy) with a struct key. -- `JPropertyKeyedCollection` lookups are dictionary-backed; `ScanFilter` descendants use an allocation-free pointer walk; JPath parses are cached via `JTokenExtensions.ParsePath`. From 813af7e498fe288e7d4a5f6de17126b6e0ab141e Mon Sep 17 00:00:00 2001 From: Simon Cropp Date: Thu, 27 Aug 2026 22:54:03 +1000 Subject: [PATCH 07/12] Update Directory.Build.props --- src/Directory.Build.props | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Directory.Build.props b/src/Directory.Build.props index af5e4425a..4a608f479 100644 --- a/src/Directory.Build.props +++ b/src/Directory.Build.props @@ -2,7 +2,7 @@ CS1591;CS1573;NU1605;NU1608;NU1109 preview - 0.35.2 + 0.36.0 1.0.0 Copyright © James Newton-King 2008, Copyright © Simon Cropp 2022 Json From 6ed25f9657bd4f87080d867d6d96dae71dcfe08c Mon Sep 17 00:00:00 2001 From: Simon Cropp Date: Thu, 27 Aug 2026 22:59:23 +1000 Subject: [PATCH 08/12] Install the pinned SDK in the docs workflow The docs job runs dotnet tool install in the repo directory, where global.json pins a preview SDK. The windows runner image does not carry it, so SDK resolution fails before the tool is installed and the job exits 155. The job has not passed in the last 60 runs, going back to July, on main and on branches alike, and always for this reason: the pin has been ahead of the image the whole time. Installing the SDK the pin asks for keeps the job working across future pin bumps rather than waiting for the runner image to catch up. Running mdsnippets locally produces no markdown changes, so the first green run has nothing to push. --- .github/workflows/on-push-do-doco.yml | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/.github/workflows/on-push-do-doco.yml b/.github/workflows/on-push-do-doco.yml index 1489e09eb..3efcdc3a5 100644 --- a/.github/workflows/on-push-do-doco.yml +++ b/.github/workflows/on-push-do-doco.yml @@ -6,6 +6,12 @@ jobs: runs-on: windows-latest steps: - uses: actions/checkout@v4 + # global.json pins a preview SDK that the runner image does not carry, so every dotnet + # command in the repo directory fails to resolve one. Install the pinned SDK rather than + # relying on what happens to be preinstalled + - uses: actions/setup-dotnet@v6 + with: + global-json-file: global.json - name: Run MarkdownSnippets run: | dotnet tool install --global MarkdownSnippets.Tool From d5adeb5040cc9d03008f8592dd18aa75d96d687b Mon Sep 17 00:00:00 2001 From: Simon Cropp Date: Thu, 27 Aug 2026 23:03:53 +1000 Subject: [PATCH 09/12] Bump checkout to v7 in the docs workflow --- .github/workflows/on-push-do-doco.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/on-push-do-doco.yml b/.github/workflows/on-push-do-doco.yml index 3efcdc3a5..24c3fd22b 100644 --- a/.github/workflows/on-push-do-doco.yml +++ b/.github/workflows/on-push-do-doco.yml @@ -5,7 +5,7 @@ jobs: docs: runs-on: windows-latest steps: - - uses: actions/checkout@v4 + - uses: actions/checkout@v7 # global.json pins a preview SDK that the runner image does not carry, so every dotnet # command in the repo directory fails to resolve one. Install the pinned SDK rather than # relying on what happens to be preinstalled From 8c8446144ce83647f9061b025fc1f852acceed76 Mon Sep 17 00:00:00 2001 From: Simon Cropp Date: Thu, 27 Aug 2026 23:09:24 +1000 Subject: [PATCH 10/12] Update milestone-release.yml --- .github/workflows/milestone-release.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/milestone-release.yml b/.github/workflows/milestone-release.yml index 55c1b9340..c954d2cbc 100644 --- a/.github/workflows/milestone-release.yml +++ b/.github/workflows/milestone-release.yml @@ -25,7 +25,7 @@ jobs: runs-on: ubuntu-latest steps: - name: Sync Release with Milestone - uses: actions/github-script@v7 + uses: actions/github-script@v9 with: script: | const { owner, repo } = context.repo; From e5556484fca6acb157fa39cb195cf9cfbfd61956 Mon Sep 17 00:00:00 2001 From: Simon Cropp Date: Fri, 28 Aug 2026 11:56:17 +1000 Subject: [PATCH 11/12] . --- src/Argon/GlobalUsings.cs | 2 ++ src/Argon/Serialization/JsonObjectContract.cs | 7 +------ src/Argon/Serialization/JsonSerializerInternalReader.cs | 6 +----- src/ArgonTests/Benchmarks/HotPathBenchmarks.cs | 4 +--- src/ArgonTests/Benchmarks/PerValueBenchmarks.cs | 2 -- 5 files changed, 5 insertions(+), 16 deletions(-) diff --git a/src/Argon/GlobalUsings.cs b/src/Argon/GlobalUsings.cs index d4e152d63..cd1c192aa 100644 --- a/src/Argon/GlobalUsings.cs +++ b/src/Argon/GlobalUsings.cs @@ -6,3 +6,5 @@ global using System.Reflection.Emit; global using System.Text.RegularExpressions; global using System.Collections.Frozen; +global using System.Runtime.CompilerServices; +global using System.Runtime.InteropServices; diff --git a/src/Argon/Serialization/JsonObjectContract.cs b/src/Argon/Serialization/JsonObjectContract.cs index 9c110c571..27c1650b7 100644 --- a/src/Argon/Serialization/JsonObjectContract.cs +++ b/src/Argon/Serialization/JsonObjectContract.cs @@ -75,12 +75,7 @@ internal int IndexOfCreatorParameter(JsonProperty property) creatorParameterIndexes = indexes; } - if (indexes.TryGetValue(property, out var result)) - { - return result; - } - - return -1; + return indexes.GetValueOrDefault(property, -1); } bool? hasRequiredOrDefaultValueProperties; diff --git a/src/Argon/Serialization/JsonSerializerInternalReader.cs b/src/Argon/Serialization/JsonSerializerInternalReader.cs index 6e469781a..4c696ba95 100644 --- a/src/Argon/Serialization/JsonSerializerInternalReader.cs +++ b/src/Argon/Serialization/JsonSerializerInternalReader.cs @@ -2,10 +2,6 @@ // Use of this source code is governed by The MIT License, // as found in the license.md file. - -#if NET6_0_OR_GREATER -using System.Runtime.CompilerServices; -using System.Runtime.InteropServices; #endif // ReSharper disable NullableWarningSuppressionIsUsed @@ -2366,4 +2362,4 @@ protected bool IsDeserializeErrorHandled(object? currentObject, object? member, return currentDeserializeErrorContext.Handled; } -} \ No newline at end of file +} diff --git a/src/ArgonTests/Benchmarks/HotPathBenchmarks.cs b/src/ArgonTests/Benchmarks/HotPathBenchmarks.cs index 4eba283ba..92a9129f6 100644 --- a/src/ArgonTests/Benchmarks/HotPathBenchmarks.cs +++ b/src/ArgonTests/Benchmarks/HotPathBenchmarks.cs @@ -2,8 +2,6 @@ // Use of this source code is governed by The MIT License, // as found in the license.md file. -using BenchmarkDotNet.Attributes; - // Benchmarks covering the hot path fixes from the performance review in todo.md. // // The reader string scan and the escape writer fixes are already measured by @@ -285,7 +283,7 @@ public void Setup() .Select(_ => _ % 9) .ToArray(); - array = new(Enumerable.Range(0, 500)); + array = [with(Enumerable.Range(0, 500))]; } [Benchmark] diff --git a/src/ArgonTests/Benchmarks/PerValueBenchmarks.cs b/src/ArgonTests/Benchmarks/PerValueBenchmarks.cs index abb0e3e20..7d1dcc74d 100644 --- a/src/ArgonTests/Benchmarks/PerValueBenchmarks.cs +++ b/src/ArgonTests/Benchmarks/PerValueBenchmarks.cs @@ -2,8 +2,6 @@ // Use of this source code is governed by The MIT License, // as found in the license.md file. -using BenchmarkDotNet.Attributes; - // Benchmarks covering the medium and low priority items from the performance review in todo.md. // The high priority items are covered by HotPathBenchmarks. From 71de7b08b3855abaef4bff180d7c9f44b9389fc2 Mon Sep 17 00:00:00 2001 From: Simon Cropp Date: Fri, 28 Aug 2026 11:56:50 +1000 Subject: [PATCH 12/12] . --- src/Argon/Serialization/JsonSerializerInternalReader.cs | 2 -- src/ArgonTests/Benchmarks/PerValueBenchmarks.cs | 2 +- src/ArgonTests/Linq/JsonPath/JPathParseTests.cs | 4 ++-- 3 files changed, 3 insertions(+), 5 deletions(-) diff --git a/src/Argon/Serialization/JsonSerializerInternalReader.cs b/src/Argon/Serialization/JsonSerializerInternalReader.cs index 4c696ba95..aa2068814 100644 --- a/src/Argon/Serialization/JsonSerializerInternalReader.cs +++ b/src/Argon/Serialization/JsonSerializerInternalReader.cs @@ -2,8 +2,6 @@ // Use of this source code is governed by The MIT License, // as found in the license.md file. -#endif - // ReSharper disable NullableWarningSuppressionIsUsed // ReSharper disable RedundantSuppressNullableWarningExpression diff --git a/src/ArgonTests/Benchmarks/PerValueBenchmarks.cs b/src/ArgonTests/Benchmarks/PerValueBenchmarks.cs index 7d1dcc74d..34206a791 100644 --- a/src/ArgonTests/Benchmarks/PerValueBenchmarks.cs +++ b/src/ArgonTests/Benchmarks/PerValueBenchmarks.cs @@ -202,7 +202,7 @@ public class NumberScanBenchmark public void Setup() { var number = Digits <= 2 - ? new string('7', Digits) + ? new('7', Digits) : $"{new string('7', Digits - 2)}.7"; json = $"[{string.Join(",", Enumerable.Repeat(number, 500))}]"; } diff --git a/src/ArgonTests/Linq/JsonPath/JPathParseTests.cs b/src/ArgonTests/Linq/JsonPath/JPathParseTests.cs index 341bd6e92..b6e2181a0 100644 --- a/src/ArgonTests/Linq/JsonPath/JPathParseTests.cs +++ b/src/ArgonTests/Linq/JsonPath/JPathParseTests.cs @@ -301,7 +301,7 @@ public void SinglePropertyAndFilterWithEscapeAtEnd() [Fact] public void SinglePropertyAndFilterWithEmptyString() { - var path = new JPath(@"Blah[ ?( @.name=='' ) ]"); + var path = new JPath("Blah[ ?( @.name=='' ) ]"); Assert.Equal(2, path.Filters.Count); var expressions = (BooleanQueryExpression) ((QueryFilter) path.Filters[1]).Expression; Assert.Equal("", (string) (JToken) expressions.Right); @@ -826,4 +826,4 @@ public void AndImmediatelyFollowedByOr() [Fact] public void RegexOperatorWithoutSlashesThrowsAtParseTime() => Assert.Throws(() => new JPath("$[?(@.name =~ 'abc')]")); -} \ No newline at end of file +}