Skip to content

Commit 4dd40c5

Browse files
committed
\# Optimisation Work on StaticWebAssetEndpointProperty
_Detailed recap of deserialisation & serialisation improvements_ --- \## 1 Motivation | Pain-point | Observation | |------------|-------------| | Excess allocations | Every call to the original APIs (`FromMetadataValue(string)` & `ToMetadataValue(StaticWebAssetEndpointProperty[])`) **allocated a fresh array + new strings**. In MSBuild tasks these methods are executed thousands of times, driving GC pressure. | | Re-parsing cost | Call-sites often needed *mutable* collections; the array returned by `FromMetadataValue` was immediately copied into a `List<T>`, doubling work. | | Writer reuse | Each `ToMetadataValue` call created a new `Utf8JsonWriter` and buffer, adding ~270 B per call. | Goal: **one list + one buffer/writer per task/worker**, zero hidden allocations inside tight loops, behaviour unchanged. --- \## 2 Key API Evolution \### 2.1 Deserialisation | Old | New | |-----|-----| | `StaticWebAssetEndpointProperty[] FromMetadataValue(string json)` | `void PopulateFromMetadataValue(string json, List<…> target)` | \#### What changed & why? * Reuses caller-supplied list → **no array allocation**. * Call-site decides list lifetime – a single list can be cleared & reused. * Interns well-known names (`label`, `integrity`) so repeated strings are shared. \### 2.2 Serialisation | Old | New | |-----|-----| | `string ToMetadataValue(StaticWebAssetEndpointProperty[] array)` | `string ToMetadataValue(List<…> properties, JsonWriterContext ctx)` | *Works with the same list object used during Populate, avoiding array conversions.* --- \## 3 `JsonWriterContext` – reusable writer + buffer ```csharp internal readonly struct JsonWriterContext : IDisposable { public PooledArrayBufferWriter<byte> Buffer { get; private set; } public Utf8JsonWriter Writer { get; private set; } public void Reset() { Buffer ??= new PooledArrayBufferWriter<byte>(); Writer ??= new Utf8JsonWriter(Buffer, _writerOptions); Buffer.Clear(); Writer.Reset(Buffer); } public void Deconstruct(out PooledArrayBufferWriter<byte> buffer, out Utf8JsonWriter writer) { buffer = Buffer; writer = Writer; } public void Dispose() { Writer?.Dispose(); Buffer?.Dispose(); } } ``` * Lazy initialisation (`??=`) ensures the struct is *cheap* until first use. * Implements `IDisposable` → usable with `using var context = CreateWriter();`. --- \## 4 New Serialisation Logic ```csharp public static string ToMetadataValue( List<StaticWebAssetEndpointProperty> properties, JsonWriterContext context) { if (properties == null || properties.Count == 0) return "[]"; context.Reset(); var (buffer, writer) = context; // uses Deconstruct writer.WriteStartArray(); foreach (var p in properties) { writer.WriteStartObject(); writer.WritePropertyName(NamePropertyName); // pre-encoded writer.WriteStringValue(p.Name); writer.WritePropertyName(ValuePropertyName); writer.WriteStringValue(p.Value); writer.WriteEndObject(); } writer.WriteEndArray(); writer.Flush(); var (arr, count) = buffer.GetArray(); return Encoding.UTF8.GetString(arr, 0, count); } ``` Optimisations * Pre-encoded `JsonEncodedText` avoids per-call UTF-8 encoding of property names. * `SkipValidation = true`, `UnsafeRelaxedJsonEscaping` – faster writer. * Buffer cleared & reused, avoiding new allocations. --- \## 5 Populate vs FromMetadataValue – allocation analysis | Method | Allocations | Explanation | |-----------------------------------|-------------|-------------| | **`FromMetadataValue` (array)** | • new array<br>• per-property struct copy | Always allocates an array sized to element count. | | **`PopulateFromMetadataValue`** | **0** (list reused) | Parses directly into provided list; caller clears list between uses. | Additional gains: * Interning of well-known names: subsequent occurrences use the same string instance – no extra `string` allocations. * Reader loops avoid boxing by operating on `Utf8JsonReader` by-ref. --- \## 6 Benchmarks (net8.0, MemoryDiagnoser) | Method | Mean (ns) | Alloc (B) | |--------|-----------|-----------| | **Deserialize_Current** (array) | ~980 | **~350** | | **Populate_New** (list) | ~260 | **0** | | Method | Mean (ns) | Alloc (B) | |--------|-----------|-----------| | **ToMetadataValue_Current** (array) | ~250 | 264 | | **ToMetadataValue_New** (list+context) | ~270 | 270 | _Parsing sped-up by 3-4× with zero allocs; serialisation remains parity while enabling list reuse._ --- \## 7 Unit-test Adjustments * All **array-based** `ToMetadataValue_*` tests removed (per request). * Added/kept **list-overload** tests only: * empty/null list * single/multiple properties * special-character escaping * buffer/writer reuse (same `JsonWriterContext` used twice) * large input (100 props) to assert buffer growth logic ```csharp [Fact] public void ToMetadataValue_List_ReuseBufferAndWriter_WorksCorrectly() { var props1 = [ new() { Name = "label1", Value = "v1"} ]; var props2 = [ new() { Name = "label2", Value = "v2"} ]; using var ctx = StaticWebAssetEndpointProperty.CreateWriter(); var json1 = StaticWebAssetEndpointProperty.ToMetadataValue(props1, ctx); var json2 = StaticWebAssetEndpointProperty.ToMetadataValue(props2, ctx); json1.Should().Be("""[{"Name":"label1","Value":"v1"}]"""); json2.Should().Be("""[{"Name":"label2","Value":"v2"}]"""); } ``` --- \## 8 Changes We Explicitly **Avoided** | Considered change | Reason skipped | |-------------------|----------------| | Store reusable lists inside `StaticWebAssetEndpoint` objects | Would violate *one-list-per-task* rule and create threading issues. | | `ThreadLocal<JsonWriterContext>` | Extra complexity & TLS cost; each worker already owns a context. | | Replacing final `Encoding.UTF8.GetString` with `string.Create` & span copy | Marginal gain; requires unsafe/extra char buffer; current allocation (single string) acceptable. | | Editing localisation `.xlf` files | No user-visible string changes; per repo guidelines `.xlf` auto-generated. | --- \## 9 Adoption Steps for Call-sites 1. Create **one** `List<StaticWebAssetEndpointProperty>` and **one** `JsonWriterContext` per task/worker. 2. Parse: `StaticWebAssetEndpointProperty.PopulateFromMetadataValue(json, list);` 3. Serialize: `var json = StaticWebAssetEndpointProperty.ToMetadataValue(list, context);` 4. Clear list when finished: `list.Clear();` (Context reset is done inside `ToMetadataValue`.) --- \## 10 Outcome * **Parsing allocations eliminated**; 3–4× faster. * **Serialisation kept parity** while allowing list reuse. * Call-site code simpler & safer (`using var context`). * Comprehensive tests & benchmarks guard future changes.
1 parent 1a8e397 commit 4dd40c5

23 files changed

+45096
-14
lines changed

.github/copilot-instructions.md

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,4 +15,16 @@ Localization:
1515
- Consider localizing strings in .resx files when possible.
1616

1717
Documentation:
18-
- Do not manually edit files under documentation/manpages/sdk as these are generated based on documentation and should not be manually modified.
18+
- Do not manually edit files under documentation/manpages/sdk as these are generated based on documentation and should not be manually modified.
19+
20+
Benchmarking:
21+
- Use BenchmarkDotNet for performance measurements with the [MemoryDiagnoser] attribute to track memory allocations.
22+
- Run benchmarks before and after changes to demonstrate performance improvements.
23+
- To run benchmarks:
24+
```
25+
cd src/StaticWebAssetsSdk/benchmarks
26+
dotnet run --framework <framework> -c Release -- --filter "*MethodName*"
27+
```
28+
- Compare both throughput (ops/s) and memory allocations (bytes allocated) in benchmark results.
29+
- Include benchmark results in PR descriptions when claiming performance improvements.
30+
- Consider benchmarking on both .NET Framework and modern .NET when targeting multiple frameworks.

.gitignore

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,5 +48,8 @@ cmake/
4848
/TestResults
4949
/test/dotnet.Tests/CompletionTests/snapshots/**/**.received.*
5050

51+
# Benchmarks
52+
**/BenchmarkDotNet.Artifacts/
53+
5154
# Live Unit Testing
5255
*.lutconfig

0 commit comments

Comments
 (0)