From bca1b799a7b2f39f025bdbb1b0b53999eb4ad79b Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 12 Oct 2026 00:42:36 +0000 Subject: [PATCH] Write ISO 8211 records straight into their buffers and merge S-57 updates without list searches No public API changes. Writer: - Work out every record's layout first, then write each record directly into an exact-size buffer. This replaces appending to a List one byte at a time, formatting every number and the leader through strings, and copying the list into the result array. - The stream and file overloads write a 64 KB chunk of records at a time from a pooled buffer instead of building the whole output first. Every record is still checked before anything is written, so a document that can't be encoded leaves the stream untouched and creates no file, as before. - Layouts no longer keep each record's field lengths and positions, which are recomputed while writing. On the synthetic harbour cell, Write allocates 592 KB instead of 2.3 MB. On the synthetic Large cell, it takes 54 ms and allocates 28 MB, down from 85 ms and 69 MB; writing to a stream allocates 14 MB. The output is byte-identical on the synthetic cells and 20 real NOAA cells and updates, through every overload and with directory auto-sizing on. ApplyChanges: - Deleting a record no longer searches the list of records for its key, and inserting one no longer checks the list for it; a delete leaves the key behind, and building the output skips it. A base cell that repeats a record name, which is malformed, falls back to the list-based merge so its output can't change. - Three updates on the synthetic Large cell take 66 ms instead of 156 ms. A randomized test compares the merge with the list-based one, including repeated names, and real NOAA update chains give identical fingerprints. S57Document.GetFeatureRecord(S57RecordName) and GetVectorRecord loop instead of building a LINQ closure per call; neither this repository nor EncDotNet.S100 calls them, so they keep searching every record and point to S57Chart for indexed lookups. Budget tests skip, with a reason, when the libraries are Debug builds, whose allocations differ from the Release figures the budgets record, and a failure names the runtime the budgets were recorded on when the run used another. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01YagvELyJKBANk61DyFGUNJ --- .../Iso8211DocumentWriter.cs | 142 ++++++++++++-- src/EncDotNet.Iso8211/Iso8211RecordBuilder.cs | 9 +- src/EncDotNet.Iso8211/Iso8211RecordWriter.cs | 185 ++++++++++-------- src/EncDotNet.S57/S57Document.cs | 126 ++++++++++-- .../PerformanceBudget.cs | 29 ++- .../PerformanceBudgets.cs | 7 +- .../Iso8211DocumentWriterTests.cs | 40 ++++ .../S57DocumentRecordMergeTests.cs | 100 ++++++++++ 8 files changed, 520 insertions(+), 118 deletions(-) create mode 100644 tests/EncDotNet.UnitTests/S57DocumentRecordMergeTests.cs diff --git a/src/EncDotNet.Iso8211/Iso8211DocumentWriter.cs b/src/EncDotNet.Iso8211/Iso8211DocumentWriter.cs index 3374531..52b469c 100644 --- a/src/EncDotNet.Iso8211/Iso8211DocumentWriter.cs +++ b/src/EncDotNet.Iso8211/Iso8211DocumentWriter.cs @@ -1,3 +1,5 @@ +using System.Buffers; + namespace EncDotNet.Iso8211; /// @@ -17,6 +19,9 @@ namespace EncDotNet.Iso8211; /// public static class Iso8211DocumentWriter { + // Stream and file output is gathered into chunks of about this size before each write. + private const int ChunkSize = 64 * 1024; + /// /// Serializes an ISO 8211 document to a byte array. /// @@ -25,16 +30,18 @@ public static class Iso8211DocumentWriter /// The serialized ISO 8211 bytes. public static byte[] Write(Iso8211Document document, Iso8211WriterOptions? options = null) { - ArgumentNullException.ThrowIfNull(document); - options ??= Iso8211WriterOptions.Default; + var plan = Plan(document, options); - var output = new List(EstimateCapacity(document)); - foreach (var record in document.Records) + var output = new byte[plan.TotalLength]; + var offset = 0; + for (int i = 0; i < plan.Layouts.Length; i++) { - Iso8211RecordWriter.Write(record, options, output); + var length = plan.Layouts[i].RecordLength; + Iso8211RecordWriter.Write(document.Records[i], plan.Layouts[i], output.AsSpan(offset, length)); + offset += length; } - return output.ToArray(); + return output; } /// @@ -43,11 +50,15 @@ public static byte[] Write(Iso8211Document document, Iso8211WriterOptions? optio /// The destination stream. /// The document to serialize. /// Optional writer options. Defaults to . + /// + /// Every record is checked before anything is written, so a document that can't be encoded + /// leaves the stream untouched. The output is then written a chunk of records at a time + /// rather than built in memory first. + /// public static void Write(Stream stream, Iso8211Document document, Iso8211WriterOptions? options = null) { ArgumentNullException.ThrowIfNull(stream); - var bytes = Write(document, options); - stream.Write(bytes, 0, bytes.Length); + WriteChunks(stream, document, Plan(document, options)); } /// @@ -58,6 +69,11 @@ public static void Write(Stream stream, Iso8211Document document, Iso8211WriterO /// Optional writer options. Defaults to . /// A token to cancel the operation. /// A task that represents the asynchronous write operation. + /// + /// Every record is checked before anything is written, so a document that can't be encoded + /// leaves the stream untouched. The output is then written a chunk of records at a time + /// rather than built in memory first. + /// public static async Task WriteAsync( Stream stream, Iso8211Document document, @@ -65,8 +81,7 @@ public static async Task WriteAsync( CancellationToken cancellationToken = default) { ArgumentNullException.ThrowIfNull(stream); - var bytes = Write(document, options); - await stream.WriteAsync(bytes, cancellationToken).ConfigureAwait(false); + await WriteChunksAsync(stream, document, Plan(document, options), cancellationToken).ConfigureAwait(false); } /// @@ -75,10 +90,16 @@ public static async Task WriteAsync( /// The destination file path. /// The document to serialize. /// Optional writer options. Defaults to . + /// + /// Every record is checked before the file is opened, so a document that can't be encoded + /// doesn't create or change the file. + /// public static void WriteToFile(string path, Iso8211Document document, Iso8211WriterOptions? options = null) { - var bytes = Write(document, options); - File.WriteAllBytes(path, bytes); + var plan = Plan(document, options); + + using var stream = new FileStream(path, FileMode.Create, FileAccess.Write, FileShare.Read, bufferSize: 1); + WriteChunks(stream, document, plan); } /// @@ -89,27 +110,106 @@ public static void WriteToFile(string path, Iso8211Document document, Iso8211Wri /// Optional writer options. Defaults to . /// A token to cancel the operation. /// A task that represents the asynchronous write operation. + /// + /// Every record is checked before the file is opened, so a document that can't be encoded + /// doesn't create or change the file. + /// public static async Task WriteToFileAsync( string path, Iso8211Document document, Iso8211WriterOptions? options = null, CancellationToken cancellationToken = default) { - var bytes = Write(document, options); - await File.WriteAllBytesAsync(path, bytes, cancellationToken).ConfigureAwait(false); + var plan = Plan(document, options); + + var stream = new FileStream(path, FileMode.Create, FileAccess.Write, FileShare.Read, bufferSize: 1, useAsync: true); + await using (stream.ConfigureAwait(false)) + { + await WriteChunksAsync(stream, document, plan, cancellationToken).ConfigureAwait(false); + } } - private static int EstimateCapacity(Iso8211Document document) + // Works out every record's layout, which also checks that each can be encoded. + private static WritePlan Plan(Iso8211Document document, Iso8211WriterOptions? options) { - var total = 0; - foreach (var record in document.Records) + ArgumentNullException.ThrowIfNull(document); + options ??= Iso8211WriterOptions.Default; + + var records = document.Records; + var layouts = new Iso8211RecordLayout[records.Count]; + long total = 0; + var max = 0; + for (int i = 0; i < layouts.Length; i++) { - total += 32 + (record.Directory.Count * 12); - foreach (var field in record.Fields) + layouts[i] = Iso8211RecordWriter.Plan(records[i], options); + total += layouts[i].RecordLength; + max = Math.Max(max, layouts[i].RecordLength); + } + + if (total > Array.MaxLength) + { + throw new InvalidOperationException($"The document encodes to {total:N0} bytes, more than an array can hold."); + } + + return new WritePlan(layouts, (int)total, max); + } + + private static void WriteChunks(Stream stream, Iso8211Document document, WritePlan plan) + { + var buffer = ArrayPool.Shared.Rent(Math.Max(ChunkSize, plan.MaxRecordLength)); + try + { + var record = 0; + while (record < plan.Layouts.Length) { - total += field.Data.Length + 1; + var length = FillChunk(document, plan, buffer, ref record); + stream.Write(buffer, 0, length); } } - return total > 0 ? total : 64; + finally + { + ArrayPool.Shared.Return(buffer); + } + } + + private static async Task WriteChunksAsync(Stream stream, Iso8211Document document, WritePlan plan, CancellationToken cancellationToken) + { + var buffer = ArrayPool.Shared.Rent(Math.Max(ChunkSize, plan.MaxRecordLength)); + try + { + var record = 0; + while (record < plan.Layouts.Length) + { + var length = FillChunk(document, plan, buffer, ref record); + await stream.WriteAsync(buffer.AsMemory(0, length), cancellationToken).ConfigureAwait(false); + } + } + finally + { + ArrayPool.Shared.Return(buffer); + } } + + // Encodes as many whole records as fit in the buffer, starting at the given record, and + // returns the number of bytes written. The buffer always holds at least the longest record. + private static int FillChunk(Iso8211Document document, WritePlan plan, byte[] buffer, ref int record) + { + var offset = 0; + while (record < plan.Layouts.Length) + { + var length = plan.Layouts[record].RecordLength; + if (offset + length > buffer.Length) + { + break; + } + + Iso8211RecordWriter.Write(document.Records[record], plan.Layouts[record], buffer.AsSpan(offset, length)); + offset += length; + record++; + } + + return offset; + } + + private readonly record struct WritePlan(Iso8211RecordLayout[] Layouts, int TotalLength, int MaxRecordLength); } diff --git a/src/EncDotNet.Iso8211/Iso8211RecordBuilder.cs b/src/EncDotNet.Iso8211/Iso8211RecordBuilder.cs index 8e6cb25..f17527b 100644 --- a/src/EncDotNet.Iso8211/Iso8211RecordBuilder.cs +++ b/src/EncDotNet.Iso8211/Iso8211RecordBuilder.cs @@ -136,15 +136,20 @@ public Iso8211Record Build() var layout = Iso8211RecordWriter.ComputeLayout(seedLeader, _fields, _options); + // Each field's length includes its terminator, and its position is the sum of the + // lengths before it, as the writer lays them out. var directory = new List(_fields.Count); + var position = 0; for (int i = 0; i < _fields.Count; i++) { + var length = _fields[i].Data.Length + 1; directory.Add(new Iso8211DirectoryEntry { Tag = _fields[i].Tag, - Length = layout.Lengths[i], - Position = layout.Positions[i] + Length = length, + Position = position }); + position += length; } var leader = seedLeader with diff --git a/src/EncDotNet.Iso8211/Iso8211RecordWriter.cs b/src/EncDotNet.Iso8211/Iso8211RecordWriter.cs index 4057ef5..fe426bc 100644 --- a/src/EncDotNet.Iso8211/Iso8211RecordWriter.cs +++ b/src/EncDotNet.Iso8211/Iso8211RecordWriter.cs @@ -37,36 +37,73 @@ internal static class Iso8211RecordWriter private const int UnspecifiedRecordLength = 0; /// - /// Encodes the specified record and appends the resulting bytes to . + /// Computes a record's layout and checks that its leader can be encoded, without emitting + /// any bytes, so a document's records can all be checked before any of them is written. /// /// The record to encode. /// The writer options. - /// The destination buffer to append the encoded record to. - public static void Write(Iso8211Record record, Iso8211WriterOptions options, List output) + /// The layout to pass to . + /// The record cannot be encoded. + public static Iso8211RecordLayout Plan(Iso8211Record record, Iso8211WriterOptions options) { ArgumentNullException.ThrowIfNull(record); ArgumentNullException.ThrowIfNull(options); - ArgumentNullException.ThrowIfNull(output); + var layout = ComputeLayout(record.Leader, record.Fields, options); + + // The field control length has two characters in the leader. + var fieldControlLength = record.Leader.FieldControlLength; + if (fieldControlLength is < 0 or > 99) + { + var encodedLength = LeaderLength - 2 + fieldControlLength.ToString("D2", CultureInfo.InvariantCulture).Length; + throw new InvalidOperationException($"Encoded leader length was {encodedLength}; expected {LeaderLength}."); + } + + return layout; + } + + /// + /// Encodes the specified record into , which must hold + /// exactly bytes. + /// + /// The record to encode. + /// The record's layout, from . + /// The bytes to write the record to. + public static void Write(Iso8211Record record, Iso8211RecordLayout layout, Span destination) + { var fields = record.Fields; - var layout = ComputeLayout(record.Leader, fields, options); - WriteLeader(output, record.Leader, layout.RecordLength, layout.BaseAddress, layout.LengthSize, layout.PositionSize, layout.TagSize); + WriteLeader(destination, record.Leader, layout.RecordLength, layout.BaseAddress, layout.LengthSize, layout.PositionSize, layout.TagSize); + var offset = LeaderLength; - // Directory. + // Directory. Each field's length includes its terminator, and its position is the sum + // of the lengths before it. + var position = 0; for (int i = 0; i < fields.Count; i++) { - WriteTag(output, fields[i].Tag, layout.TagSize); - WriteNumeric(output, layout.Lengths[i], layout.LengthSize); - WriteNumeric(output, layout.Positions[i], layout.PositionSize); + var length = fields[i].Data.Length + 1; + WriteTag(destination.Slice(offset, layout.TagSize), fields[i].Tag); + offset += layout.TagSize; + WriteNumeric(destination.Slice(offset, layout.LengthSize), length); + offset += layout.LengthSize; + WriteNumeric(destination.Slice(offset, layout.PositionSize), position); + offset += layout.PositionSize; + position += length; } - output.Add(FieldTerminator); + destination[offset++] = FieldTerminator; // Field area. for (int i = 0; i < fields.Count; i++) { - output.AddRange(fields[i].Data.Span); - output.Add(FieldTerminator); + var data = fields[i].Data.Span; + data.CopyTo(destination[offset..]); + offset += data.Length; + destination[offset++] = FieldTerminator; + } + + if (offset != layout.RecordLength) + { + throw new InvalidOperationException($"Encoded record length was {offset}; expected {layout.RecordLength}."); } } @@ -87,21 +124,21 @@ public static Iso8211RecordLayout ComputeLayout( Iso8211WriterOptions options) { var fieldCount = fields.Count; - var fieldLengths = new int[fieldCount]; - var fieldPositions = new int[fieldCount]; var fieldAreaLength = 0; + var maxLength = 0; + var maxPosition = 0; for (int i = 0; i < fieldCount; i++) { var length = fields[i].Data.Length + 1; // +1 for the field terminator - fieldLengths[i] = length; - fieldPositions[i] = fieldAreaLength; + maxLength = Math.Max(maxLength, length); + maxPosition = Math.Max(maxPosition, fieldAreaLength); fieldAreaLength += length; } var tagSize = DetermineTagSize(leader, fields, options); - var lengthSize = DetermineNumericSize(leader.SizeOfFieldLengthField, fieldLengths, options); - var positionSize = DetermineNumericSize(leader.SizeOfFieldPositionField, fieldPositions, options); + var lengthSize = DetermineNumericSize(leader.SizeOfFieldLengthField, maxLength, options); + var positionSize = DetermineNumericSize(leader.SizeOfFieldPositionField, maxPosition, options); var entrySize = tagSize + lengthSize + positionSize; var directoryBytes = entrySize * fieldCount; @@ -114,11 +151,11 @@ public static Iso8211RecordLayout ComputeLayout( var recordLength = baseAddress + fieldAreaLength; - return new Iso8211RecordLayout(fieldLengths, fieldPositions, tagSize, lengthSize, positionSize, baseAddress, recordLength); + return new Iso8211RecordLayout(tagSize, lengthSize, positionSize, baseAddress, recordLength); } private static void WriteLeader( - List output, + Span output, Iso8211RecordLeader leader, int recordLength, int baseAddress, @@ -126,31 +163,30 @@ private static void WriteLeader( int positionSize, int tagSize) { - var sb = new StringBuilder(LeaderLength); + Span text = stackalloc char[LeaderLength]; + // A record length beyond five digits is written as "00000"; the reader recovers it // from the directory. - var leaderRecordLength = recordLength > MaxLeaderNumericValue ? UnspecifiedRecordLength : recordLength; - sb.Append(leaderRecordLength.ToString("D5", CultureInfo.InvariantCulture)); - sb.Append(OrDefault(leader.InterchangeLevel, '3')); - sb.Append(OrDefault(leader.LeaderIdentifier, 'D')); - sb.Append(OrDefault(leader.InlineCodeExtensionIndicator, 'E')); - sb.Append(OrDefault(leader.VersionNumber, '1')); - sb.Append(OrDefault(leader.ApplicationIndicator, ' ')); - sb.Append(leader.FieldControlLength.ToString("D2", CultureInfo.InvariantCulture)); - sb.Append(baseAddress.ToString("D5", CultureInfo.InvariantCulture)); - sb.Append(FitExtendedCharset(leader.ExtendedCharacterSetIndicator)); - sb.Append((char)('0' + lengthSize)); - sb.Append((char)('0' + positionSize)); - sb.Append('0'); // Reserved. - sb.Append((char)('0' + tagSize)); - - var leaderText = sb.ToString(); - if (leaderText.Length != LeaderLength) + FormatDigits(text[..5], recordLength > MaxLeaderNumericValue ? UnspecifiedRecordLength : recordLength); + text[5] = OrDefault(leader.InterchangeLevel, '3'); + text[6] = OrDefault(leader.LeaderIdentifier, 'D'); + text[7] = OrDefault(leader.InlineCodeExtensionIndicator, 'E'); + text[8] = OrDefault(leader.VersionNumber, '1'); + text[9] = OrDefault(leader.ApplicationIndicator, ' '); + FormatDigits(text.Slice(10, 2), leader.FieldControlLength); + FormatDigits(text.Slice(12, 5), baseAddress); + FitExtendedCharset(text.Slice(17, 3), leader.ExtendedCharacterSetIndicator); + text[20] = (char)('0' + lengthSize); + text[21] = (char)('0' + positionSize); + text[22] = '0'; // Reserved. + text[23] = (char)('0' + tagSize); + + // ASCII, with '?' for anything outside it, as the leader has always been encoded. + var written = Encoding.ASCII.GetBytes(text, output[..LeaderLength]); + if (written != LeaderLength) { - throw new InvalidOperationException($"Encoded leader length was {leaderText.Length}; expected {LeaderLength}."); + throw new InvalidOperationException($"Encoded leader length was {written}; expected {LeaderLength}."); } - - output.AddRange(Encoding.ASCII.GetBytes(leaderText)); } private static int DetermineTagSize(Iso8211RecordLeader leader, IReadOnlyList fields, Iso8211WriterOptions options) @@ -177,17 +213,10 @@ private static int DetermineTagSize(Iso8211RecordLeader leader, IReadOnlyList required) - { - required = digits; - } - } + var required = maxValue == 0 ? 1 : (int)Math.Floor(Math.Log10(maxValue)) + 1; if (!options.AutoSizeDirectoryEntries && declaredSize > 0) { @@ -202,44 +231,48 @@ private static int DetermineNumericSize(int declaredSize, int[] values, Iso8211W return required; } - private static void WriteTag(List output, string tag, int tagSize) + private static void WriteTag(Span output, string tag) { tag ??= string.Empty; - for (int i = 0; i < tagSize; i++) + for (int i = 0; i < output.Length; i++) { - output.Add(i < tag.Length ? (byte)tag[i] : (byte)' '); + output[i] = i < tag.Length ? (byte)tag[i] : (byte)' '; } } - private static void WriteNumeric(List output, int value, int size) + // Writes a non-negative value as decimal digits, zero-padded to fill the output. + private static void WriteNumeric(Span output, int value) { - var text = value.ToString(CultureInfo.InvariantCulture); - if (text.Length > size) + var remaining = value; + for (int i = output.Length - 1; i >= 0; i--) { - throw new InvalidOperationException($"Value {value} does not fit in {size} digit(s)."); + output[i] = (byte)('0' + (remaining % 10)); + remaining /= 10; } - for (int i = 0; i < size - text.Length; i++) + + if (remaining != 0) { - output.Add((byte)'0'); + throw new InvalidOperationException($"Value {value} does not fit in {output.Length} digit(s)."); } - for (int i = 0; i < text.Length; i++) + } + + // Writes a value from 0 to the width's maximum as zero-padded decimal digits. + private static void FormatDigits(Span output, int value) + { + for (int i = output.Length - 1; i >= 0; i--) { - output.Add((byte)text[i]); + output[i] = (char)('0' + (value % 10)); + value /= 10; } } - private static string FitExtendedCharset(string? value) + private static void FitExtendedCharset(Span output, string? value) { value ??= " "; - if (value.Length == 3) - { - return value; - } - if (value.Length > 3) + for (int i = 0; i < output.Length; i++) { - return value.Substring(0, 3); + output[i] = i < value.Length ? value[i] : ' '; } - return value.PadRight(3, ' '); } private static char OrDefault(char value, char fallback) => value == '\0' ? fallback : value; @@ -251,16 +284,12 @@ private static string FitExtendedCharset(string? value) internal readonly struct Iso8211RecordLayout { public Iso8211RecordLayout( - int[] lengths, - int[] positions, int tagSize, int lengthSize, int positionSize, int baseAddress, int recordLength) { - Lengths = lengths; - Positions = positions; TagSize = tagSize; LengthSize = lengthSize; PositionSize = positionSize; @@ -268,12 +297,6 @@ public Iso8211RecordLayout( RecordLength = recordLength; } - /// Gets the per-field lengths, including each field's trailing terminator. - public int[] Lengths { get; } - - /// Gets the per-field positions within the field area. - public int[] Positions { get; } - /// Gets the size of the field tag field in directory entries. public int TagSize { get; } diff --git a/src/EncDotNet.S57/S57Document.cs b/src/EncDotNet.S57/S57Document.cs index e58eaaf..e0425ba 100644 --- a/src/EncDotNet.S57/S57Document.cs +++ b/src/EncDotNet.S57/S57Document.cs @@ -1,4 +1,6 @@ using System.Collections.Immutable; +using System.Runtime.CompilerServices; +using System.Runtime.InteropServices; namespace EncDotNet.S57; @@ -45,21 +47,41 @@ public sealed record S57Document /// /// Gets a feature record by its record name. /// + /// + /// This searches every feature record. To look up many records, build an + /// , whose lookups use an index. + /// public S57FeatureRecord? GetFeatureRecord(S57RecordName name) { - return FeatureRecords.FirstOrDefault(r => - r.RecordName.RecordNameCode == name.RecordNameCode && - r.RecordName.RecordId == name.RecordId); + foreach (var record in FeatureRecords) + { + if (SameRecord(record.RecordName, name)) + { + return record; + } + } + + return null; } /// /// Gets a vector record by its record name. /// + /// + /// This searches every vector record. To look up many records, build an + /// , whose lookups use an index. + /// public S57VectorRecord? GetVectorRecord(S57RecordName name) { - return VectorRecords.FirstOrDefault(r => - r.RecordName.RecordNameCode == name.RecordNameCode && - r.RecordName.RecordId == name.RecordId); + foreach (var record in VectorRecords) + { + if (SameRecord(record.RecordName, name)) + { + return record; + } + } + + return null; } /// @@ -147,8 +169,89 @@ private static ImmutableArray ApplyRecordChanges( Func applyModify) where T : class { - // Build a mutable dictionary keyed by (RCNM, RCID) for efficient lookup. - // Use a list to preserve insertion order for records not touched by updates. + // Records by (RCNM, RCID), and their keys in output order. A delete leaves its key in + // the order rather than searching the list for it; the output skips keys that no + // longer have a record. A key inserted again after a delete is appended at its new + // position, and only that position outputs its record. + var records = new Dictionary<(int, int), T>(baseRecords.Count); + var order = new List<(int, int)>(baseRecords.Count); + HashSet<(int, int)>? deleted = null; + Dictionary<(int, int), int>? reinsertedAt = null; + + foreach (var record in baseRecords) + { + var key = getKey(record); + if (!records.TryAdd(key, record)) + { + // A base that repeats a record name is malformed. Merge it as the list-based + // merge always has, so its output doesn't change. + return ApplyRecordChangesToRepeatedNames(baseRecords, updateRecords, getKey, applyModify); + } + + order.Add(key); + } + + foreach (var updateRecord in updateRecords) + { + var key = getKey(updateRecord); + var instruction = GetUpdateInstruction(updateRecord); + + switch (instruction) + { + case S57UpdateInstruction.Insert: + ref var inserted = ref CollectionsMarshal.GetValueRefOrAddDefault(records, key, out var exists); + inserted = updateRecord; + if (!exists) + { + if (deleted is not null && deleted.Contains(key)) + { + (reinsertedAt ??= [])[key] = order.Count; + } + + order.Add(key); + } + break; + + case S57UpdateInstruction.Delete: + if (records.Remove(key)) + { + (deleted ??= []).Add(key); + } + break; + + case S57UpdateInstruction.Modify: + ref var modified = ref CollectionsMarshal.GetValueRefOrNullRef(records, key); + if (!Unsafe.IsNullRef(ref modified)) + { + modified = applyModify(modified, updateRecord); + } + break; + } + } + + var builder = ImmutableArray.CreateBuilder(records.Count); + for (var position = 0; position < order.Count; position++) + { + var key = order[position]; + if (records.TryGetValue(key, out var record) && + (reinsertedAt is null || !reinsertedAt.TryGetValue(key, out var current) || current == position)) + { + builder.Add(record); + } + } + + return builder.MoveToImmutable(); + } + + // The list-based merge, kept for bases that repeat a record name: each appearance of a + // name outputs its record, and a delete removes only the name's first appearance. + private static ImmutableArray ApplyRecordChangesToRepeatedNames( + IReadOnlyList baseRecords, + IReadOnlyList updateRecords, + Func getKey, + Func applyModify) + where T : class + { var dict = new Dictionary<(int, int), T>(baseRecords.Count); var orderedKeys = new List<(int, int)>(baseRecords.Count); @@ -162,9 +265,7 @@ private static ImmutableArray ApplyRecordChanges( foreach (var updateRecord in updateRecords) { var key = getKey(updateRecord); - var instruction = GetUpdateInstruction(updateRecord); - - switch (instruction) + switch (GetUpdateInstruction(updateRecord)) { case S57UpdateInstruction.Insert: dict[key] = updateRecord; @@ -200,6 +301,9 @@ private static ImmutableArray ApplyRecordChanges( return builder.ToImmutable(); } + private static bool SameRecord(S57RecordName a, S57RecordName b) => + a.RecordNameCode == b.RecordNameCode && a.RecordId == b.RecordId; + private static S57UpdateInstruction GetUpdateInstruction(T record) => record switch { S57FeatureRecord f => f.UpdateInstruction, diff --git a/tests/EncDotNet.PerformanceTests/PerformanceBudget.cs b/tests/EncDotNet.PerformanceTests/PerformanceBudget.cs index 9b76781..de96ad6 100644 --- a/tests/EncDotNet.PerformanceTests/PerformanceBudget.cs +++ b/tests/EncDotNet.PerformanceTests/PerformanceBudget.cs @@ -1,3 +1,6 @@ +using System.Diagnostics; +using System.Reflection; + namespace EncDotNet.PerformanceTests; /// @@ -48,6 +51,13 @@ private static void AssertWithin(string scenario, string kind, long actual, long { TestContext.Current.TestOutputHelper?.WriteLine($"{scenario}: {actual:N0} bytes {kind} (budget {budget:N0})"); + if (!LibrariesAreOptimized) + { + Assert.Skip( + "The budgets are recorded from Release builds, and Debug builds of the libraries allocate " + + "differently. Run these tests with --configuration Release, as CI does."); + } + // Budgets under 1 KB are checked exactly; a percentage of zero is no tolerance at all. var upper = budget < 1024 ? budget : (long)(budget * (1 + over)); var lower = budget < 1024 ? budget : (long)(budget * (1 - under)); @@ -57,17 +67,32 @@ private static void AssertWithin(string scenario, string kind, long actual, long Assert.Fail( $"{scenario} {kind} {actual:N0} bytes, over its budget of {budget:N0} by {Percent(actual, budget)}. " + "Find the regression, or if the increase is intended, raise the budget in PerformanceBudgets.cs " + - "and explain why in the pull request."); + "and explain why in the pull request." + RuntimeNote()); } if (actual < lower) { Assert.Fail( $"{scenario} {kind} {actual:N0} bytes, under its budget of {budget:N0} by {Percent(actual, budget)}. " + - $"Lower the budget in PerformanceBudgets.cs to {actual:N0} to lock in the improvement."); + $"Lower the budget in PerformanceBudgets.cs to {actual:N0} to lock in the improvement." + RuntimeNote()); } } + // Whether the libraries under test were compiled with optimizations, as in Release builds. + private static bool LibrariesAreOptimized { get; } = + IsOptimized(typeof(Iso8211.Iso8211Reader).Assembly) && IsOptimized(typeof(S57.S57Document).Assembly); + + private static bool IsOptimized(Assembly assembly) => + assembly.GetCustomAttribute()?.IsJITOptimizerDisabled != true; + + // A different .NET runtime can compile the same code to allocate differently, so a + // failure says when the run didn't use the runtime the budgets were recorded on. + private static string RuntimeNote() => + Environment.Version.ToString() == PerformanceBudgets.RecordedOnRuntime + ? string.Empty + : $" The budgets were recorded on .NET {PerformanceBudgets.RecordedOnRuntime}, and this run used " + + $".NET {Environment.Version}; if every budget moved, re-record them for the new runtime."; + // The size of the difference as a share of the budget. private static string Percent(long actual, long budget) => budget == 0 ? "∞" : $"{Math.Abs(actual - budget) * 100.0 / budget:F1}%"; diff --git a/tests/EncDotNet.PerformanceTests/PerformanceBudgets.cs b/tests/EncDotNet.PerformanceTests/PerformanceBudgets.cs index 943054f..a2d3c55 100644 --- a/tests/EncDotNet.PerformanceTests/PerformanceBudgets.cs +++ b/tests/EncDotNet.PerformanceTests/PerformanceBudgets.cs @@ -16,6 +16,11 @@ namespace EncDotNet.PerformanceTests; /// internal static class PerformanceBudgets { + /// + /// The .NET runtime the budgets were recorded on. Update it when re-recording the budgets. + /// + public const string RecordedOnRuntime = "10.0.12"; + // Allocated bytes for one operation. public const long Iso8211ReaderAllTokens = 0; public const long Iso8211DocumentReaderRead = 2_088_976; @@ -23,7 +28,7 @@ internal static class PerformanceBudgets public const long Iso8211DataDescriptiveRecordReaderRead = 32_808; public const long Iso8211FieldReaderCoordinates = 846_376; public const long Iso8211FieldReaderCoordinatesByOrdinal = 141_264; - public const long Iso8211DocumentWriterWrite = 2_318_456; + public const long Iso8211DocumentWriterWrite = 591_616; public const long S57DocumentReaderRead = 3_713_184; public const long S57DocumentApplyChanges = 685_440; public const long S57ChartFromDocument = 1_063_752; diff --git a/tests/EncDotNet.UnitTests/Iso8211DocumentWriterTests.cs b/tests/EncDotNet.UnitTests/Iso8211DocumentWriterTests.cs index 4c1c6be..9dff2ca 100644 --- a/tests/EncDotNet.UnitTests/Iso8211DocumentWriterTests.cs +++ b/tests/EncDotNet.UnitTests/Iso8211DocumentWriterTests.cs @@ -1,5 +1,6 @@ using System.Text; using EncDotNet.Iso8211; +using EncDotNet.Testing; namespace EncDotNet.UnitTests; @@ -277,6 +278,45 @@ public async Task WriteAsync_And_WriteToFile_ProduceSameBytes() } } + [Fact] + public async Task WriteToStream_DocumentLargerThanOneChunk_MatchesByteArray() + { + // The harbour cell is about 400 KB, so the stream overloads write it in several chunks. + var document = Iso8211DocumentReader.Read(SyntheticS57Cell.Create(SyntheticCellProfile.Harbour)); + var expected = Iso8211DocumentWriter.Write(document); + + using var sync = new MemoryStream(); + Iso8211DocumentWriter.Write(sync, document); + using var async = new MemoryStream(); + await Iso8211DocumentWriter.WriteAsync(async, document, cancellationToken: TestContext.Current.CancellationToken); + + Assert.True(expected.Length > 256 * 1024); + Assert.Equal(expected, sync.ToArray()); + Assert.Equal(expected, async.ToArray()); + } + + [Fact] + public async Task WriteToStreamOrFile_RecordThatCannotBeEncoded_WritesNothing() + { + // The second record's field control length doesn't fit the leader's two characters, + // so the document can't be written; the first record must not be written either. + var valid = Iso8211DocumentReader.Read(BuildRecordBytes('D', new[] { ("0001", "TEST"u8.ToArray()) })).Records[0]; + var invalid = valid with { Leader = valid.Leader with { FieldControlLength = 100 } }; + var document = new Iso8211Document { Records = [valid, invalid] }; + + using var stream = new MemoryStream(); + Assert.Throws(() => Iso8211DocumentWriter.Write(stream, document)); + await Assert.ThrowsAsync( + () => Iso8211DocumentWriter.WriteAsync(stream, document, cancellationToken: TestContext.Current.CancellationToken)); + Assert.Equal(0, stream.Length); + + var path = Path.Combine(Path.GetTempPath(), $"{Guid.NewGuid():N}.000"); + Assert.Throws(() => Iso8211DocumentWriter.WriteToFile(path, document)); + await Assert.ThrowsAsync( + () => Iso8211DocumentWriter.WriteToFileAsync(path, document, cancellationToken: TestContext.Current.CancellationToken)); + Assert.False(File.Exists(path)); + } + [Fact] public void Write_ThenReadFromFile_RoundTrips() { diff --git a/tests/EncDotNet.UnitTests/S57DocumentRecordMergeTests.cs b/tests/EncDotNet.UnitTests/S57DocumentRecordMergeTests.cs new file mode 100644 index 0000000..c623ac8 --- /dev/null +++ b/tests/EncDotNet.UnitTests/S57DocumentRecordMergeTests.cs @@ -0,0 +1,100 @@ +using System.Collections.Immutable; +using EncDotNet.S57; + +namespace EncDotNet.UnitTests; + +/// +/// Checks that orders the merged records exactly as its +/// earlier list-based merge did, including for base cells that repeat a record name. +/// +public class S57DocumentRecordMergeTests +{ + [Fact] + public void ApplyChanges_RandomUpdates_MatchListBasedMerge() + { + var random = new Random(8211); + for (var trial = 0; trial < 2_000; trial++) + { + var source = 0; + var keySpace = random.Next(1, 8); + + // A base cell, sometimes repeating a record name. + var baseRecords = new List(); + for (var i = random.Next(0, 8); i > 0; i--) + { + baseRecords.Add(Feature(random.Next(keySpace), ++source, version: 1, S57UpdateInstruction.Insert)); + } + + var updateRecords = new List(); + for (var i = random.Next(0, 12); i > 0; i--) + { + var instruction = (S57UpdateInstruction)random.Next(1, 4); + updateRecords.Add(Feature(random.Next(keySpace), ++source, version: 2 + i, instruction)); + } + + var document = new S57Document { FeatureRecords = baseRecords, VectorRecords = [] }; + var actual = document.ApplyChanges(new S57Document { FeatureRecords = updateRecords, VectorRecords = [] }) + .FeatureRecords + .Select(r => (r.RecordName.RecordId, (int)r.ObjectCode, r.RecordVersion)); + + Assert.Equal(ListBasedMerge(baseRecords, updateRecords), actual); + } + } + + // The merge as it was before it counted appearances: a list of keys searched with + // Contains and Remove. A modify keeps the base record's object code and takes the + // update's version, as ApplyChanges does. + private static List<(int, int, int)> ListBasedMerge(List baseRecords, List updateRecords) + { + var records = new Dictionary(); + var order = new List(); + foreach (var r in baseRecords) + { + records[r.RecordName.RecordId] = ((int)r.ObjectCode, r.RecordVersion); + order.Add(r.RecordName.RecordId); + } + + foreach (var r in updateRecords) + { + var key = r.RecordName.RecordId; + switch (r.UpdateInstruction) + { + case S57UpdateInstruction.Insert: + records[key] = ((int)r.ObjectCode, r.RecordVersion); + if (!order.Contains(key)) + { + order.Add(key); + } + + break; + case S57UpdateInstruction.Delete: + records.Remove(key); + order.Remove(key); + break; + case S57UpdateInstruction.Modify: + if (records.TryGetValue(key, out var existing)) + { + records[key] = (existing.ObjectCode, r.RecordVersion); + } + + break; + } + } + + return [.. order.Where(records.ContainsKey).Select(k => (k, records[k].ObjectCode, records[k].Version))]; + } + + private static S57FeatureRecord Feature(int rcid, int source, int version, S57UpdateInstruction instruction) => new() + { + RecordName = S57RecordName.FromRcnmRcid(S57RecordNameCodes.Feature, rcid), + Primitive = S57GeometricPrimitive.None, + Group = 2, + ObjectCode = (S57ObjectCode)source, + RecordVersion = version, + UpdateInstruction = instruction, + Attributes = ImmutableArray.Empty, + NationalAttributes = ImmutableArray.Empty, + SpatialPointers = ImmutableArray.Empty, + FeaturePointers = ImmutableArray.Empty, + }; +}