From e6e208eebed0dd46d68c143e6a754218a7fff138 Mon Sep 17 00:00:00 2001 From: Marc Sallin Date: Sun, 6 Sep 2026 21:05:51 +0200 Subject: [PATCH] perf: stop allocating a warnings array for a clean compile Every document allocated a string array and wrapped it with Array.AsReadOnly, including the common case of no warnings at all, where both are 24 bytes of pure ceremony. The shared empty collection covers that case without allocating and keeps Warnings immutable, which a caller-facing list has to stay. --- src/typstsharp.tests/Tests.cs | 20 ++++++++++++++++++++ src/typstsharp/TypstDocument.cs | 29 +++++++++++++++++++++-------- 2 files changed, 41 insertions(+), 8 deletions(-) diff --git a/src/typstsharp.tests/Tests.cs b/src/typstsharp.tests/Tests.cs index 4a5b967..441df45 100644 --- a/src/typstsharp.tests/Tests.cs +++ b/src/typstsharp.tests/Tests.cs @@ -910,6 +910,26 @@ public async Task WarningsFromADocumentCannotBeMutatedByCallers() await Assert.That(document.Warnings is string[]).IsFalse(); } + /// + /// A clean compile takes a different branch from one that reports warnings, so the empty case + /// needs the same guarantees: enumerable, and not a mutable array handed out to callers. + /// + [Test] + public async Task WarningsFromACleanCompileAreEmptyAndStillImmutable() + { + using var compiler = TypstCompiler.FromSource("= No warnings here"); + using var document = compiler.CompileToDocument(); + + await Assert.That(document.Warnings.Count).IsEqualTo(0); + await Assert.That(document.Warnings is string[]).IsFalse(); + await Assert.That(document.Warnings.Any()).IsFalse(); + + document.Dispose(); + + // Warnings are copied out of native memory eagerly, so they outlive the document. + await Assert.That(document.Warnings.Count).IsEqualTo(0); + } + private const string TwoPageSource = """ First page #pagebreak() diff --git a/src/typstsharp/TypstDocument.cs b/src/typstsharp/TypstDocument.cs index 5813764..e04953f 100644 --- a/src/typstsharp/TypstDocument.cs +++ b/src/typstsharp/TypstDocument.cs @@ -1,4 +1,5 @@ using System.Buffers; +using System.Collections.ObjectModel; using Microsoft.Win32.SafeHandles; namespace typstsharp; @@ -52,14 +53,26 @@ internal unsafe TypstDocument(CsBindgen.CompileResult native) throw new InvalidOperationException("The Typst compiler reported warnings but returned none."); } - // Warnings are small and are copied eagerly so that they stay usable after disposal. - var warnings = new string[warningCount]; - for (int i = 0; i < warnings.Length; i++) + // Warnings are small and are copied eagerly so that they stay usable after disposal. A + // clean compile is the common case and reaches the shared empty collection, which spares + // every such document both the zero-length array and the wrapper around it. + IReadOnlyList warnings; + if (warningCount == 0) { - var warning = native.warnings[i]; - warnings[i] = warning.message_ptr != null - ? System.Text.Encoding.UTF8.GetString(new ReadOnlySpan(warning.message_ptr, checked((int)warning.message_len))) - : string.Empty; + warnings = ReadOnlyCollection.Empty; + } + else + { + var collected = new string[warningCount]; + for (int i = 0; i < collected.Length; i++) + { + var warning = native.warnings[i]; + collected[i] = warning.message_ptr != null + ? System.Text.Encoding.UTF8.GetString(new ReadOnlySpan(warning.message_ptr, checked((int)warning.message_len))) + : string.Empty; + } + + warnings = Array.AsReadOnly(collected); } long nativeByteCount = 0; @@ -69,7 +82,7 @@ internal unsafe TypstDocument(CsBindgen.CompileResult native) } _outputCount = outputCount; - _warnings = Array.AsReadOnly(warnings); + _warnings = warnings; _nativeByteCount = nativeByteCount; // Taking ownership must be the last thing that happens. An object with a finalizer is queued