Skip to content

Commit a148b15

Browse files
Copilotpetesramek
andauthored
Change Encode parameter from ReadOnlySpan<TValue> to IEnumerable<TValue> (#236)
- [x] Understand current PolylineEncoder.cs structure - [x] Replace `IReadOnlyList<TValue>` materialization with direct `foreach` + `anyItemProcessed` flag - [x] Replace pre-allocated char buffer with grow-on-demand (`stackalloc char[StackAllocLimit]` → double-and-rent from `ArrayPool` on overflow) - [x] Replace heap `long[]` scratch arrays with `stackalloc long[MaxStackWidth]` sliced to `[..width]` - [x] Factor shared encode logic into private `EncodeCore` (both overloads delegate to it) - [x] Update `SeedPrevious` to accept `Span<long>` instead of `long[]` - [x] Remove `GetMaxBufferLength` - [x] Address code review: rename `hasItems` → `anyItemProcessed` for clarity - [x] Build clean (0 new warnings); all 225 tests pass; 0 CodeQL alerts --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: petesramek <2333452+petesramek@users.noreply.github.com>
1 parent 1b87336 commit a148b15

8 files changed

Lines changed: 105 additions & 146 deletions

File tree

benchmarks/PolylineAlgorithm.Benchmarks/PolylineEncoderBenchmark.cs

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ namespace PolylineAlgorithm.Benchmarks;
44
using BenchmarkDotNet.Engines;
55
using PolylineAlgorithm;
66
using PolylineAlgorithm.Utility;
7+
using System.Collections.Generic;
78

89
/// <summary>
910
/// Benchmarks for <see cref="PolylineEncoder{TValue, TPolyline}"/>.
@@ -24,9 +25,9 @@ public class PolylineEncoderBenchmark {
2425
public (double Latitude, double Longitude)[] Array { get; private set; }
2526

2627
/// <summary>
27-
/// Coordinates as read-only memory.
28+
/// Coordinates as list.
2829
/// </summary>
29-
public ReadOnlyMemory<(double Latitude, double Longitude)> Memory { get; private set; }
30+
public List<(double Latitude, double Longitude)> List { get; private set; }
3031

3132
#pragma warning restore CS8618 // Non-nullable field must contain a non-null value when exiting constructor. Consider adding the 'required' modifier or declaring as nullable.
3233

@@ -53,15 +54,15 @@ public class PolylineEncoderBenchmark {
5354
[GlobalSetup]
5455
public void SetupData() {
5556
Array = [.. RandomValueProvider.GetCoordinates(CoordinatesCount)];
56-
Memory = Array.AsMemory();
57+
List = [.. RandomValueProvider.GetCoordinates(CoordinatesCount)];
5758
}
5859

5960
/// <summary>
60-
/// Benchmark: encode coordinates from span.
61+
/// Benchmark: encode coordinates from list.
6162
/// </summary>
6263
[Benchmark]
63-
public void PolylineEncoder_Encode_Span() {
64-
var polyline = _encoder.Encode(Memory.Span);
64+
public void PolylineEncoder_Encode_List() {
65+
var polyline = _encoder.Encode(List);
6566
_consumer.Consume(polyline);
6667
}
6768

src/PolylineAlgorithm/Abstraction/IPolylineEncoder.cs

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,10 @@
44
//
55

66
namespace PolylineAlgorithm.Abstraction;
7+
8+
using System.Collections.Generic;
9+
using System.Threading;
10+
711
/// <summary>
812
/// Contract for encoding a sequence of values into an encoded polyline representation.
913
/// Implementations interpret the generic <typeparamref name="TValue"/> type and produce an encoded
@@ -30,18 +34,17 @@ namespace PolylineAlgorithm.Abstraction;
3034
/// - Value precision and rounding rules (for example 1e-5 for 5-decimal precision).
3135
/// - Value ordering and whether altitude or additional dimensions are supported.
3236
/// - Thread-safety guarantees: whether instances are safe to reuse concurrently or must be instantiated per-call.
33-
/// - Implementations are encouraged to be memory-efficient; the API accepts a <see cref="ReadOnlySpan{T}"/>
34-
/// to avoid forced allocations when callers already have contiguous memory.
3537
/// </remarks>
3638
public interface IPolylineEncoder<TValue, TPolyline> {
3739
/// <summary>
3840
/// Encodes a sequence of values into an encoded polyline representation.
3941
/// The order of values in <paramref name="coordinates"/> is preserved in the encoded result.
4042
/// </summary>
4143
/// <param name="coordinates">
42-
/// The collection of <typeparamref name="TValue"/> instances to encode into a polyline.
43-
/// The span may be empty; implementations should return an appropriate empty encoded representation
44+
/// The sequence of <typeparamref name="TValue"/> instances to encode into a polyline.
45+
/// The sequence may be empty; implementations should return an appropriate empty encoded representation
4446
/// (for example an empty string or an empty memory slice) rather than <see langword="null"/>.
47+
/// Must not be <see langword="null"/>.
4548
/// </param>
4649
/// <param name="cancellationToken">
4750
/// A <see cref="System.Threading.CancellationToken"/> that can be used to cancel the encoding operation.
@@ -70,8 +73,11 @@ public interface IPolylineEncoder<TValue, TPolyline> {
7073
/// - For large input sequences, implementations may provide streaming or incremental encoders; those
7174
/// variants can still implement this interface by materializing the final encoded result.
7275
/// </remarks>
76+
/// <exception cref="System.ArgumentNullException">
77+
/// Thrown if <paramref name="coordinates"/> is <see langword="null"/>.
78+
/// </exception>
7379
/// <exception cref="System.OperationCanceledException">
7480
/// Thrown if the operation is canceled via <paramref name="cancellationToken"/>.
7581
/// </exception>
76-
TPolyline Encode(ReadOnlySpan<TValue> coordinates, PolylineEncodingOptions<TValue>? options = null, CancellationToken cancellationToken = default);
82+
TPolyline Encode(IEnumerable<TValue> coordinates, PolylineEncodingOptions<TValue>? options = null, CancellationToken cancellationToken = default);
7783
}

src/PolylineAlgorithm/Extensions/PolylineEncoderExtensions.cs

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,8 @@ namespace PolylineAlgorithm.Extensions;
77

88
using PolylineAlgorithm.Abstraction;
99
using PolylineAlgorithm.Internal.Diagnostics;
10-
using System;
10+
using System.Collections.Generic;
11+
using System.Threading;
1112

1213
/// <summary>
1314
/// Provides extension methods for the <see cref="IPolylineEncoder{TValue, TPolyline}"/> interface to facilitate encoding values into polylines.
@@ -27,7 +28,7 @@ public static class PolylineEncoderExtensions {
2728
/// <returns>
2829
/// A <typeparamref name="TPolyline"/> instance representing the encoded polyline for the provided values.
2930
/// </returns>
30-
/// <exception cref="ArgumentNullException">
31+
/// <exception cref="System.ArgumentNullException">
3132
/// Thrown when <paramref name="encoder"/> or <paramref name="values"/> is <see langword="null"/>.
3233
/// </exception>
3334
public static TPolyline Encode<TValue, TPolyline>(
@@ -43,6 +44,6 @@ public static TPolyline Encode<TValue, TPolyline>(
4344
ExceptionGuard.ThrowArgumentNull(nameof(values));
4445
}
4546

46-
return encoder.Encode(values.AsSpan(), options, cancellationToken);
47+
return encoder.Encode(values, options, cancellationToken);
4748
}
4849
}

src/PolylineAlgorithm/PolylineEncoder.cs

Lines changed: 58 additions & 109 deletions
Original file line numberDiff line numberDiff line change
@@ -7,12 +7,10 @@ namespace PolylineAlgorithm;
77

88
using Microsoft.Extensions.Logging;
99
using PolylineAlgorithm.Abstraction;
10-
using PolylineAlgorithm.Internal;
1110
using PolylineAlgorithm.Internal.Diagnostics;
1211
using System;
1312
using System.Buffers;
14-
using System.Diagnostics;
15-
using System.Runtime.CompilerServices;
13+
using System.Collections.Generic;
1614
using System.Threading;
1715

1816
/// <summary>
@@ -60,73 +58,24 @@ public PolylineEncoder(PolylineOptions<TValue, TPolyline> options) {
6058
/// <returns>
6159
/// An instance of <typeparamref name="TPolyline"/> representing the encoded values.
6260
/// </returns>
61+
/// <exception cref="ArgumentNullException">
62+
/// Thrown when <paramref name="coordinates"/> is <see langword="null"/>.
63+
/// </exception>
6364
/// <exception cref="ArgumentException">
6465
/// Thrown when <paramref name="coordinates"/> is empty.
6566
/// </exception>
66-
/// <exception cref="InvalidOperationException">
67-
/// Thrown when the internal encoding buffer cannot accommodate the encoded value.
68-
/// </exception>
6967
/// <exception cref="OperationCanceledException">
7068
/// Thrown when <paramref name="cancellationToken"/> is canceled.
7169
/// </exception>
72-
public TPolyline Encode(ReadOnlySpan<TValue> coordinates, CancellationToken cancellationToken = default) {
73-
const string OperationName = nameof(Encode);
74-
75-
_logger.LogOperationStartedDebug(OperationName);
76-
77-
Debug.Assert(coordinates.Length >= 0, "Count must be non-negative.");
70+
[System.Diagnostics.CodeAnalysis.SuppressMessage("Design", "CA1062:Validate arguments of public methods", Justification = "Null is verified before use via ExceptionGuard.ThrowArgumentNull, which is annotated [DoesNotReturn]. CA1062 does not recognise custom [DoesNotReturn] helpers as null guards.")]
71+
public TPolyline Encode(IEnumerable<TValue> coordinates, CancellationToken cancellationToken = default) {
72+
_logger.LogOperationStartedDebug(nameof(Encode));
7873

79-
if (coordinates.Length < 1) {
80-
_logger.LogOperationFailedDebug(OperationName);
81-
_logger.LogEmptyArgumentWarning(nameof(coordinates));
82-
ExceptionGuard.ThrowArgumentCannotBeEmptyEnumerationMessage(nameof(coordinates));
74+
if (coordinates is null) {
75+
ExceptionGuard.ThrowArgumentNull(nameof(coordinates));
8376
}
8477

85-
int width = _formatter.Width;
86-
int length = GetMaxBufferLength(coordinates.Length, width);
87-
88-
char[]? temp = length <= _options.StackAllocLimit
89-
? null
90-
: ArrayPool<char>.Shared.Rent(length);
91-
92-
Span<char> buffer = temp is null ? stackalloc char[length] : temp.AsSpan(0, length);
93-
94-
int position = 0;
95-
long[] previous = new long[width];
96-
long[] values = new long[width];
97-
98-
SeedPrevious(previous, null);
99-
100-
try {
101-
for (int i = 0; i < coordinates.Length; i++) {
102-
cancellationToken.ThrowIfCancellationRequested();
103-
104-
_formatter.GetValues(coordinates[i], values.AsSpan());
105-
106-
for (int j = 0; j < width; j++) {
107-
long current = values[j];
108-
long delta = current - previous[j];
109-
previous[j] = current;
110-
111-
if (!PolylineEncoding.TryWriteValue(delta, buffer, ref position)) {
112-
_logger.LogOperationFailedDebug(OperationName);
113-
_logger.LogCannotWriteValueToBufferWarning(position, i);
114-
ExceptionGuard.ThrowCouldNotWriteEncodedValueToBuffer();
115-
}
116-
}
117-
}
118-
119-
// Convert to string inside the try block so the buffer is still valid.
120-
string encodedResult = buffer[..position].ToString();
121-
122-
_logger.LogOperationFinishedDebug(OperationName);
123-
124-
return _formatter.Write(encodedResult.AsMemory());
125-
} finally {
126-
if (temp is not null) {
127-
ArrayPool<char>.Shared.Return(temp);
128-
}
129-
}
78+
return EncodeCore(coordinates, null, cancellationToken);
13079
}
13180

13281
/// <summary>
@@ -140,101 +89,101 @@ public TPolyline Encode(ReadOnlySpan<TValue> coordinates, CancellationToken canc
14089
/// Per-call options that control the starting delta baseline. Pass <see langword="null"/> or an
14190
/// instance with <see cref="PolylineEncodingOptions{TValue}.Previous"/> set to
14291
/// <see langword="null"/> to use the formatter's default baseline (same as calling
143-
/// <see cref="Encode(ReadOnlySpan{TValue}, CancellationToken)"/>).
92+
/// <see cref="Encode(IEnumerable{TValue}, CancellationToken)"/>).
14493
/// </param>
14594
/// <param name="cancellationToken">A token that can be used to cancel the operation.</param>
14695
/// <returns>
14796
/// An instance of <typeparamref name="TPolyline"/> representing the encoded values.
14897
/// </returns>
98+
/// <exception cref="ArgumentNullException">
99+
/// Thrown when <paramref name="coordinates"/> is <see langword="null"/>.
100+
/// </exception>
149101
/// <exception cref="ArgumentException">
150102
/// Thrown when <paramref name="coordinates"/> is empty.
151103
/// </exception>
152-
/// <exception cref="InvalidOperationException">
153-
/// Thrown when the internal encoding buffer cannot accommodate the encoded value.
154-
/// </exception>
155104
/// <exception cref="OperationCanceledException">
156105
/// Thrown when <paramref name="cancellationToken"/> is canceled.
157106
/// </exception>
158-
public TPolyline Encode(ReadOnlySpan<TValue> coordinates, PolylineEncodingOptions<TValue>? options, CancellationToken cancellationToken) {
159-
const string OperationName = nameof(Encode);
107+
[System.Diagnostics.CodeAnalysis.SuppressMessage("Design", "CA1062:Validate arguments of public methods", Justification = "Null is verified before use via ExceptionGuard.ThrowArgumentNull, which is annotated [DoesNotReturn]. CA1062 does not recognise custom [DoesNotReturn] helpers as null guards.")]
108+
public TPolyline Encode(IEnumerable<TValue> coordinates, PolylineEncodingOptions<TValue>? options, CancellationToken cancellationToken) {
109+
_logger.LogOperationStartedDebug(nameof(Encode));
160110

161-
_logger.LogOperationStartedDebug(OperationName);
111+
if (coordinates is null) {
112+
ExceptionGuard.ThrowArgumentNull(nameof(coordinates));
113+
}
162114

163-
Debug.Assert(coordinates.Length >= 0, "Count must be non-negative.");
115+
return EncodeCore(coordinates, options, cancellationToken);
116+
}
164117

165-
if (coordinates.Length < 1) {
166-
_logger.LogOperationFailedDebug(OperationName);
167-
_logger.LogEmptyArgumentWarning(nameof(coordinates));
168-
ExceptionGuard.ThrowArgumentCannotBeEmptyEnumerationMessage(nameof(coordinates));
169-
}
118+
private TPolyline EncodeCore(IEnumerable<TValue> coordinates, PolylineEncodingOptions<TValue>? options, CancellationToken cancellationToken) {
119+
const string OperationName = nameof(Encode);
120+
const int MaxStackWidth = 8;
170121

171122
int width = _formatter.Width;
172-
int length = GetMaxBufferLength(coordinates.Length, width);
173123

174-
char[]? temp = length <= _options.StackAllocLimit
175-
? null
176-
: ArrayPool<char>.Shared.Rent(length);
124+
Span<long> previous = width <= MaxStackWidth ? stackalloc long[MaxStackWidth] : new long[width];
125+
Span<long> values = width <= MaxStackWidth ? stackalloc long[MaxStackWidth] : new long[width];
126+
previous = previous[..width];
127+
values = values[..width];
177128

178-
Span<char> buffer = temp is null ? stackalloc char[length] : temp.AsSpan(0, length);
129+
SeedPrevious(previous, options);
179130

180-
int position = 0;
181-
long[] previous = new long[width];
182-
long[] values = new long[width];
131+
int stackLimit = _options.StackAllocLimit;
132+
Span<char> buffer = stackalloc char[stackLimit];
133+
char[]? rented = null;
183134

184-
SeedPrevious(previous, options);
135+
int position = 0;
136+
bool anyItemProcessed = false;
185137

186138
try {
187-
for (int i = 0; i < coordinates.Length; i++) {
139+
foreach (TValue item in coordinates) {
188140
cancellationToken.ThrowIfCancellationRequested();
189-
190-
_formatter.GetValues(coordinates[i], values.AsSpan());
141+
anyItemProcessed = true;
142+
_formatter.GetValues(item, values);
191143

192144
for (int j = 0; j < width; j++) {
193145
long current = values[j];
194146
long delta = current - previous[j];
195147
previous[j] = current;
196148

197-
if (!PolylineEncoding.TryWriteValue(delta, buffer, ref position)) {
198-
_logger.LogOperationFailedDebug(OperationName);
199-
_logger.LogCannotWriteValueToBufferWarning(position, i);
200-
ExceptionGuard.ThrowCouldNotWriteEncodedValueToBuffer();
149+
while (!PolylineEncoding.TryWriteValue(delta, buffer, ref position)) {
150+
int newSize = rented is null ? stackLimit * 2 : rented.Length * 2;
151+
char[] newRented = ArrayPool<char>.Shared.Rent(newSize);
152+
buffer[..position].CopyTo(newRented);
153+
if (rented is not null) {
154+
ArrayPool<char>.Shared.Return(rented);
155+
}
156+
rented = newRented;
157+
buffer = rented.AsSpan();
201158
}
202159
}
203160
}
204161

162+
if (!anyItemProcessed) {
163+
_logger.LogOperationFailedDebug(OperationName);
164+
_logger.LogEmptyArgumentWarning(nameof(coordinates));
165+
ExceptionGuard.ThrowArgumentCannotBeEmptyEnumerationMessage(nameof(coordinates));
166+
}
167+
205168
string encodedResult = buffer[..position].ToString();
206169

207170
_logger.LogOperationFinishedDebug(OperationName);
208171

209172
return _formatter.Write(encodedResult.AsMemory());
210173
} finally {
211-
if (temp is not null) {
212-
ArrayPool<char>.Shared.Return(temp);
174+
if (rented is not null) {
175+
ArrayPool<char>.Shared.Return(rented);
213176
}
214177
}
215178
}
216179

217-
private void SeedPrevious(long[] previous, PolylineEncodingOptions<TValue>? options) {
218-
int width = _formatter.Width;
219-
180+
private void SeedPrevious(Span<long> previous, PolylineEncodingOptions<TValue>? options) {
220181
if (options is { HasPrevious: true }) {
221-
_formatter.GetValues(options.Previous, previous.AsSpan());
182+
_formatter.GetValues(options.Previous, previous);
222183
} else {
223-
for (int j = 0; j < width; j++) {
184+
for (int j = 0; j < previous.Length; j++) {
224185
previous[j] = _formatter.GetBaseline(j);
225186
}
226187
}
227188
}
228-
229-
[MethodImpl(MethodImplOptions.AggressiveInlining)]
230-
private static int GetMaxBufferLength(int count, int valuesPerItem) {
231-
Debug.Assert(count > 0, "Count must be greater than zero.");
232-
Debug.Assert(valuesPerItem > 0, "Values per item must be greater than zero.");
233-
234-
int requestedBufferLength = count * valuesPerItem * Defaults.Polyline.Block.Length.Max;
235-
236-
Debug.Assert(requestedBufferLength > 0, "Requested buffer length must be greater than zero.");
237-
238-
return requestedBufferLength;
239-
}
240189
}
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
#nullable enable
2+
PolylineAlgorithm.PolylineEncoder<TValue, TPolyline>.Encode(System.Collections.Generic.IEnumerable<TValue>! coordinates, PolylineAlgorithm.PolylineEncodingOptions<TValue>? options, System.Threading.CancellationToken cancellationToken) -> TPolyline
3+
PolylineAlgorithm.PolylineEncoder<TValue, TPolyline>.Encode(System.Collections.Generic.IEnumerable<TValue>! coordinates, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) -> TPolyline

tests/PolylineAlgorithm.Tests/Abstraction/AbstractPolylineDecoderTests.cs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -223,7 +223,7 @@ public void Decode_With_Previous_Seeds_Accumulated_State() {
223223
(double Lat, double Lon)[] chunkB = [(43.252, -126.453)];
224224

225225
string polylineB = encoder.Encode(
226-
chunkB.AsSpan(),
226+
chunkB,
227227
new PolylineEncodingOptions<(double Lat, double Lon)>(chunkA[^1]),
228228
CancellationToken.None);
229229

@@ -269,9 +269,9 @@ public void Decode_RoundTrip_Reproduces_Full_Sequence() {
269269
(double Lat, double Lon)[] chunkB = all[2..];
270270

271271
// Encode chunked
272-
string polylineA = encoder.Encode(chunkA.AsSpan());
272+
string polylineA = encoder.Encode(chunkA);
273273
string polylineB = encoder.Encode(
274-
chunkB.AsSpan(),
274+
chunkB,
275275
new PolylineEncodingOptions<(double Lat, double Lon)>(chunkA[^1]),
276276
CancellationToken.None);
277277

0 commit comments

Comments
 (0)