You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@lucenenet.apache.org by "paulirwin (via GitHub)" <gi...@apache.org> on 2026/06/11 21:29:20 UTC
[PR] Implement IBufferWriter in OpenStringBuilder and CharTermAttribute, #1315 [lucenenet]
paulirwin opened a new pull request, #1339:
URL: https://github.com/apache/lucenenet/pull/1339
- [X] You've read the [Contributor Guide](https://github.com/apache/lucenenet/blob/master/CONTRIBUTING.md) and [Code of Conduct](https://www.apache.org/foundation/policies/conduct.html).
- [X] You've included unit or integration tests for your change, where applicable.
- [X] You've included inline docs for your change, where applicable.
- [X] There's an open issue for the PR that you are making. If you'd like to propose a change, please [open an issue](https://github.com/apache/lucenenet/issues/new/choose) to discuss the change or find an existing issue.
Implement `IBufferWriter<char>` in OpenStringBuilder and CharTermAttribute.
Fixes #1315
## Description
This PR does the following:
1. Implements `IBufferWriter<char>` in OpenStringBuilder. This uses the existing `EnsureCapacity` for the buffer-growing logic. A new `FreeCapacity` property was added to help with unit testing as well as consumers of this new API.
2. Adds `AsSpan()` and `AsMemory()` to OpenStringBuilder. This is not only useful for unit testing, but can open up some other performance improvement opportunities by reducing allocations. (Note that `ICharTermAttribute` already has `MemoryExtensions` for this.)
3. Updates `ICUNormalizer2Filter` to use the simpler `AsSpan()` calls on the attribute, and to use the span-based `Append` overload to optimize appending to the attribute.
4. Adds `IBufferWriter<char>` to `ICharTermAttribute`. **Breaking change.**
5. Adds an `Append(ReadOnlySpan<char>)` overload to `ICharTermAttribute`. **Breaking change.** This is in lieu of adding `ISpanAppendable`, per the discussion in #1315.
6. Implements `IBufferWriter<char>` in `CharTermAttribute`. This uses the existing `InternalResizeBuffer` for the buffer-growing logic. New `FreeCapacity` and `Capacity` properties were added to help with unit testing as well as consumers of this new API.
7. `CharTermAttribute` now explicitly implements the new `Append(ReadOnlySpan<char>)` overload by delegating to its existing implementation.
8. Full unit tests (where applicable) were added, ported from .NET's ArrayBufferWriter and adapted to these two types.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@lucenenet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Implement IBufferWriter in OpenStringBuilder and CharTermAttribute, #1315 [lucenenet]
Posted by "paulirwin (via GitHub)" <gi...@apache.org>.
paulirwin commented on code in PR #1339:
URL: https://github.com/apache/lucenenet/pull/1339#discussion_r3406528232
##########
src/Lucene.Net.Tests.Analysis.Common/Support/Analysis/Util/TestOpenStringBuilderBufferWriter.cs:
##########
@@ -0,0 +1,507 @@
+// Adapted from MIT-licensed ArrayBufferWriter tests here:
+// https://github.com/dotnet/runtime/blob/v10.0.6/src/libraries/System.Memory/tests/ArrayBufferWriter/ArrayBufferWriterTests.T.cs
+
+using Lucene.Net.Attributes;
+using Lucene.Net.Util;
+using NUnit.Framework;
+using System;
+using System.Buffers;
+using System.Collections.Generic;
+using System.Diagnostics;
+using System.Runtime.CompilerServices;
+using System.Runtime.InteropServices;
+
+namespace Lucene.Net.Analysis.Util
+{
+ /*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+ /// <summary>
+ /// Tests for <see cref="OpenStringBuilder"/>'s <see cref="IBufferWriter{T}"/> implementation.
+ /// </summary>
+ [TestFixture]
+ [LuceneNetSpecific]
+ public class TestOpenStringBuilderBufferWriter : LuceneTestCase
+ {
+ private const char DefaultChar = (char)0;
+ private const int DefaultBufferSize = 32; // copied from hardcoded value in OpenStringBuilder
+
+ // LUCENENET specific: works around a .NET Framework x64 RyuJIT bug where
+ // ReadOnlyMemory<char>.Span returns an empty span when the property is read
+ // inline on a local inside a large method (it only reproduces with the
+ // System.Memory polyfill; modern .NET is unaffected). Forcing the .Span read
+ // into its own non-inlined method makes the JIT compile it correctly. These
+ // helpers preserve the original assertion intent (comparing the Memory view
+ // against the Span view) on all target frameworks.
+ [MethodImpl(MethodImplOptions.NoInlining)]
+ private static bool SpanEqualsMemory(ReadOnlySpan<char> span, ReadOnlyMemory<char> memory)
+ => span.SequenceEqual(memory.Span);
+
+ [MethodImpl(MethodImplOptions.NoInlining)]
+ private static bool MemoryEqualsMemory(ReadOnlyMemory<char> a, ReadOnlyMemory<char> b)
+ => a.Span.SequenceEqual(b.Span);
+
+ [MethodImpl(MethodImplOptions.NoInlining)]
+ private static char MemoryElementAt(ReadOnlyMemory<char> memory, int index)
+ => memory.Span[index];
+
+ [Test]
+ public void ArrayBufferWriter_Ctor()
+ {
+ {
+ var output = new OpenStringBuilder();
+ Assert.AreEqual(DefaultBufferSize, output.FreeCapacity);
+ Assert.AreEqual(DefaultBufferSize, output.Capacity);
+ Assert.AreEqual(0, output.Length);
+ Assert.True(ReadOnlySpan<char>.Empty.SequenceEqual(output.AsSpan()));
+ Assert.True(MemoryEqualsMemory(ReadOnlyMemory<char>.Empty, output.AsMemory()));
+ }
+
+ {
+ var output = new OpenStringBuilder(200);
+ Assert.True(output.FreeCapacity >= 200);
+ Assert.True(output.Capacity >= 200);
+ Assert.AreEqual(0, output.Length);
+ Assert.True(ReadOnlySpan<char>.Empty.SequenceEqual(output.AsSpan()));
+ Assert.True(MemoryEqualsMemory(ReadOnlyMemory<char>.Empty, output.AsMemory()));
+ }
+
+ {
+ OpenStringBuilder output = default;
+ Assert.Null(output);
+ }
+ }
+
+ [Test]
+ public void Invalid_Ctor()
+ {
+ // LUCENENET specific - changed to ArgumentOutOfRangeException
+ Assert.Throws<ArgumentOutOfRangeException>(() => new OpenStringBuilder(0));
+ Assert.Throws<ArgumentOutOfRangeException>(() => new OpenStringBuilder(-1));
+ Assert.Throws<OutOfMemoryException>(() => new OpenStringBuilder(int.MaxValue));
+ }
+
+ [Test]
+ public void Reset()
+ {
+ var output = new OpenStringBuilder(256);
+ int previousAvailable = output.FreeCapacity;
+ WriteData(output, 2);
+ Assert.True(output.FreeCapacity < previousAvailable);
+ Assert.True(output.Length > 0);
+ Assert.False(ReadOnlySpan<char>.Empty.SequenceEqual(output.AsSpan()));
+ Assert.False(MemoryEqualsMemory(ReadOnlyMemory<char>.Empty, output.AsMemory()));
+ Assert.True(SpanEqualsMemory(output.AsSpan(), output.AsMemory()));
+
+ ReadOnlyMemory<char> transientMemory = output.AsMemory();
+ ReadOnlySpan<char> transientSpan = output.AsSpan();
+ char t0 = MemoryElementAt(transientMemory, 0);
+ char t1 = transientSpan[1];
+ Assert.AreNotEqual(DefaultChar, t0);
+ Assert.AreNotEqual(DefaultChar, t1);
+ output.Reset();
+ Assert.AreEqual(t0, MemoryElementAt(transientMemory, 0));
+ Assert.AreEqual(t1, transientSpan[1]);
+
+ Assert.AreEqual(0, output.Length);
+ Assert.True(ReadOnlySpan<char>.Empty.SequenceEqual(output.AsSpan()));
+ Assert.True(MemoryEqualsMemory(ReadOnlyMemory<char>.Empty, output.AsMemory()));
+ Assert.AreEqual(previousAvailable, output.FreeCapacity);
+ }
+
+ [Test]
+ public void Advance()
+ {
+ {
+ var output = new OpenStringBuilder();
+ int capacity = output.Capacity;
+ Assert.AreEqual(capacity, output.FreeCapacity);
+ output.Advance(output.FreeCapacity);
+ Assert.AreEqual(capacity, output.Length);
+ Assert.AreEqual(0, output.FreeCapacity);
+ }
+
+ {
+ var output = new OpenStringBuilder();
+ output.Advance(output.Capacity);
+ Assert.AreEqual(output.Capacity, output.Length);
+ Assert.AreEqual(0, output.FreeCapacity);
+ int previousCapacity = output.Capacity;
+ Span<char> _ = output.GetSpan();
+ Assert.True(output.Capacity > previousCapacity);
+ }
+
+ {
+ var output = new OpenStringBuilder(256);
+ WriteData(output, 2);
+ ReadOnlyMemory<char> previousMemory = output.AsMemory();
+ ReadOnlySpan<char> previousSpan = output.AsSpan();
+ Assert.True(SpanEqualsMemory(previousSpan, previousMemory));
+ output.Advance(10);
+ Assert.False(MemoryEqualsMemory(previousMemory, output.AsMemory()));
+ Assert.False(previousSpan.SequenceEqual(output.AsSpan()));
+ Assert.True(SpanEqualsMemory(output.AsSpan(), output.AsMemory()));
+ }
+
+ {
+ var output = new OpenStringBuilder();
+ _ = output.GetSpan(20);
+ WriteData(output, 10);
+ ReadOnlyMemory<char> previousMemory = output.AsMemory();
+ ReadOnlySpan<char> previousSpan = output.AsSpan();
+ Assert.True(SpanEqualsMemory(previousSpan, previousMemory));
+ Assert.Throws<InvalidOperationException>(() => output.Advance(247));
+ output.Advance(10);
+ Assert.False(MemoryEqualsMemory(previousMemory, output.AsMemory()));
+ Assert.False(previousSpan.SequenceEqual(output.AsSpan()));
+ Assert.True(SpanEqualsMemory(output.AsSpan(), output.AsMemory()));
+ }
+ }
+
+ [Test]
+ public void AdvanceZero()
+ {
+ var output = new OpenStringBuilder();
+ WriteData(output, 2);
+ Assert.AreEqual(2, output.Length);
+ ReadOnlyMemory<char> previousMemory = output.AsMemory();
+ ReadOnlySpan<char> previousSpan = output.AsSpan();
+ Assert.True(SpanEqualsMemory(previousSpan, previousMemory));
+ output.Advance(0);
+ Assert.AreEqual(2, output.Length);
+ Assert.True(MemoryEqualsMemory(previousMemory, output.AsMemory()));
+ Assert.True(previousSpan.SequenceEqual(output.AsSpan()));
+ Assert.True(SpanEqualsMemory(output.AsSpan(), output.AsMemory()));
+ }
+
+ [Test]
+ public void InvalidAdvance()
+ {
+ {
+ var output = new OpenStringBuilder();
+ Assert.Throws<ArgumentOutOfRangeException>(() => output.Advance(-1));
+ Assert.Throws<InvalidOperationException>(() => output.Advance(output.Capacity + 1));
+ }
+
+ {
+ var output = new OpenStringBuilder();
+ WriteData(output, 100);
+ Assert.Throws<InvalidOperationException>(() => output.Advance(output.FreeCapacity + 1));
+ }
+ }
+
+ [Test]
+ public void GetSpan_DefaultCtor()
+ {
+ var output = new OpenStringBuilder();
+ Span<char> span = output.GetSpan();
+ Assert.AreEqual(DefaultBufferSize, span.Length);
+ }
+
+ [Test]
+ [TestCaseSource(nameof(SizeHints))]
+ public void GetSpan_DefaultCtor_WithSizeHint(int sizeHint)
+ {
+ var output = new OpenStringBuilder();
+ Span<char> span = output.GetSpan(sizeHint);
+ Assert.AreEqual(sizeHint <= DefaultBufferSize ? DefaultBufferSize : sizeHint, span.Length);
+ }
+
+ [Test]
+ public void GetSpan_InitSizeCtor()
+ {
+ var output = new OpenStringBuilder(100);
+ Span<char> span = output.GetSpan();
+ Assert.AreEqual(100, span.Length);
+ }
+
+ [Test]
+ [TestCaseSource(nameof(SizeHints))]
+ public void GetSpan_InitSizeCtor_WithSizeHint(int sizeHint)
+ {
+ {
+ var output = new OpenStringBuilder(256);
+ Span<char> span = output.GetSpan(sizeHint);
+ // LUCENENET specific: due to our oversize logic, changed to GreaterOrEqual and removed sizeHint addend
+ Assert.GreaterOrEqual(span.Length, sizeHint <= 256 ? 256 : sizeHint);
+ }
+
+ {
+ var output = new OpenStringBuilder(1000);
+ Span<char> span = output.GetSpan(sizeHint);
+ // LUCENENET specific: due to our oversize logic, changed to GreaterOrEqual and removed sizeHint addend
+ Assert.GreaterOrEqual(span.Length, sizeHint <= 1000 ? 1000 : sizeHint);
+ }
+ }
+
+ [Test]
+ public void GetMemory_DefaultCtor()
+ {
+ var output = new OpenStringBuilder();
+ Memory<char> memory = output.GetMemory();
+ Assert.AreEqual(DefaultBufferSize, memory.Length);
+ }
+
+ [Test]
+ [TestCaseSource(nameof(SizeHints))]
+ public void GetMemory_DefaultCtor_WithSizeHint(int sizeHint)
+ {
+ var output = new OpenStringBuilder();
+ Memory<char> memory = output.GetMemory(sizeHint);
+ Assert.AreEqual(sizeHint <= DefaultBufferSize ? DefaultBufferSize : sizeHint, memory.Length);
+ }
+
+ [Test]
+ public void GetMemory_ExceedMaximumBufferSize_WithSmallStartingSize()
+ {
+ var output = new OpenStringBuilder(256);
+ Assert.Throws<OutOfMemoryException>(() => output.GetMemory(int.MaxValue));
+ }
+
+ [Test]
+ public void GetMemory_InitSizeCtor()
+ {
+ var output = new OpenStringBuilder(100);
+ Memory<char> memory = output.GetMemory();
+ Assert.AreEqual(100, memory.Length);
+ }
+
+ [Test]
+ [TestCaseSource(nameof(SizeHints))]
+ public void GetMemory_InitSizeCtor_WithSizeHint(int sizeHint)
+ {
+ {
+ var output = new OpenStringBuilder(256);
+ Memory<char> memory = output.GetMemory(sizeHint);
+ // LUCENENET specific: due to our oversize logic, changed to GreaterOrEqual and removed sizeHint addend
+ Assert.GreaterOrEqual(memory.Length, sizeHint <= 256 ? 256 : sizeHint);
+ }
+
+ {
+ var output = new OpenStringBuilder(1000);
+ Memory<char> memory = output.GetMemory(sizeHint);
+ // LUCENENET specific: due to our oversize logic, changed to GreaterOrEqual and removed sizeHint addend
+ Assert.GreaterOrEqual(memory.Length, sizeHint <= 1000 ? 1000 : sizeHint);
+ }
+ }
+
+ // LUCENENET specific: This test allocates a very large buffer to verify that Advance()
+ // performs its bounds check purely arithmetically (m_len vs. m_buf.Length); Advance() itself
+ // never touches buffer memory, and WriteData only touches the first 1,000 chars. If the
+ // allocation fails with OutOfMemoryException, the test is a no-op. Marked [Slow] due to the
+ // large allocation.
+ //
+ // NOTE: The upstream ArrayBufferWriter test gated this on Windows/macOS, and so do we. The
+ // risk is real on Linux but comes from the allocation itself, not from Advance(): under memory
+ // overcommit the kernel may grant new char[2_000_000_000] optimistically, then the GC's
+ // zero-initialization of the array touches every page, which can invoke the OOM killer (an
+ // uncatchable SIGKILL, not an OutOfMemoryException) and tear down the whole test run.
+ [Test]
+ [Slow]
+ public void InvalidAdvance_Large()
+ {
+ // LUCENENET specific: skip on Linux (see note above); the large allocation can be killed
+ // by the OOM killer with an uncatchable SIGKILL rather than a catchable OutOfMemoryException.
+ Assume.That(!Constants.LINUX, "Skipped on Linux: the large allocation may trigger the OOM killer.");
+
+ try
+ {
+ {
+ var output = new OpenStringBuilder(2_000_000_000);
+ WriteData(output, 1_000);
+ Assert.Throws<InvalidOperationException>(() => output.Advance(int.MaxValue));
+ Assert.Throws<InvalidOperationException>(() => output.Advance(2_000_000_000 - 1_000 + 1));
+ }
+ }
+ catch (OutOfMemoryException) { }
+ }
+
+ [Test]
+ public void GetMemoryAndSpan()
+ {
+ {
+ var output = new OpenStringBuilder();
+ WriteData(output, 2);
+ Span<char> span = output.GetSpan();
+ Memory<char> memory = output.GetMemory();
+ Span<char> memorySpan = memory.Span;
+ Assert.True(span.Length > 0);
+ Assert.True(memorySpan.Length > 0);
+ Assert.AreEqual(span.Length, memorySpan.Length);
+ for (int i = 0; i < span.Length; i++)
+ {
+ Assert.AreEqual(DefaultChar, span[i]);
+ Assert.AreEqual(DefaultChar, memorySpan[i]);
+ }
+ }
+
+ {
+ var output = new OpenStringBuilder();
+ WriteData(output, 2);
+ ReadOnlyMemory<char> writtenSoFarMemory = output.AsMemory();
+ ReadOnlySpan<char> writtenSoFar = output.AsSpan();
+ Assert.True(SpanEqualsMemory(writtenSoFar, writtenSoFarMemory));
+ int previousAvailable = output.FreeCapacity;
+ Span<char> span = output.GetSpan(500);
+ Assert.True(span.Length >= 500);
+ Assert.True(output.FreeCapacity >= 500);
+ Assert.True(output.FreeCapacity > previousAvailable);
+
+ Assert.AreEqual(writtenSoFar.Length, output.Length);
+ Assert.False(writtenSoFar.SequenceEqual(span.Slice(0, output.Length)));
+
+ Memory<char> memory = output.GetMemory();
+ Span<char> memorySpan = memory.Span;
+ Assert.True(span.Length >= 500);
+ Assert.True(memorySpan.Length >= 500);
+ Assert.AreEqual(span.Length, memorySpan.Length);
+ for (int i = 0; i < span.Length; i++)
+ {
+ Assert.AreEqual(DefaultChar, span[i]);
+ Assert.AreEqual(DefaultChar, memorySpan[i]);
+ }
+
+ memory = output.GetMemory(500);
+ memorySpan = memory.Span;
+ Assert.True(memorySpan.Length >= 500);
+ Assert.AreEqual(span.Length, memorySpan.Length);
+ for (int i = 0; i < memorySpan.Length; i++)
+ {
+ Assert.AreEqual(DefaultChar, memorySpan[i]);
+ }
+ }
+ }
+
+ [Test]
+ public void GetSpanShouldAtleastDoubleWhenGrowing()
+ {
+ var output = new OpenStringBuilder(256);
+ WriteData(output, 100);
+ int previousAvailable = output.FreeCapacity;
+
+ _ = output.GetSpan(previousAvailable);
+ Assert.AreEqual(previousAvailable, output.FreeCapacity);
+
+ _ = output.GetSpan(previousAvailable + 1);
+ Assert.True(output.FreeCapacity >= previousAvailable * 2);
+ }
+
+ [Test]
+ public void GetSpanOnlyGrowsAboveThreshold()
+ {
+ {
+ var output = new OpenStringBuilder();
+ _ = output.GetSpan();
+ int previousAvailable = output.FreeCapacity;
+
+ for (int i = 0; i < 10; i++)
+ {
+ _ = output.GetSpan();
+ Assert.AreEqual(previousAvailable, output.FreeCapacity);
+ }
+ }
+
+ {
+ var output = new OpenStringBuilder();
+ _ = output.GetSpan(10);
+ int previousAvailable = output.FreeCapacity;
+
+ for (int i = 0; i < 10; i++)
+ {
+ _ = output.GetSpan(previousAvailable);
+ Assert.AreEqual(previousAvailable, output.FreeCapacity);
+ }
+ }
+ }
+
+ [Test]
+ public void InvalidGetMemoryAndSpan()
+ {
+ var output = new OpenStringBuilder();
+ WriteData(output, 2);
+ Assert.Throws<ArgumentOutOfRangeException>(() => output.GetSpan(-1));
+ Assert.Throws<ArgumentOutOfRangeException>(() => output.GetMemory(-1));
+ }
+
+ [Test]
+ public void MultipleCallsToGetSpan()
+ {
+ var output = new OpenStringBuilder(300);
+ Assert.True(MemoryMarshal.TryGetArray(output.GetMemory(), out ArraySegment<char> array));
+ GCHandle pinnedArray = GCHandle.Alloc(array.Array, GCHandleType.Pinned);
+ try
+ {
+ int previousAvailable = output.FreeCapacity;
+ Assert.True(previousAvailable >= 300);
+ Assert.True(output.Capacity >= 300);
+ Assert.AreEqual(previousAvailable, output.Capacity);
+ Span<char> span = output.GetSpan();
+ Assert.True(span.Length >= previousAvailable);
+ Assert.True(span.Length >= 256);
+ Span<char> newSpan = output.GetSpan();
+ Assert.AreEqual(span.Length, newSpan.Length);
+ // LUCENENET specific: changed expected to IntPtr.Zero due to presumed Xunit vs. NUnit differences
+ Assert.AreEqual(IntPtr.Zero, Unsafe.ByteOffset(ref MemoryMarshal.GetReference(span), ref MemoryMarshal.GetReference(newSpan)));
+ Assert.AreEqual(span.Length, output.GetSpan().Length);
+ }
+ finally
+ {
+ pinnedArray.Free();
+ }
+ }
+
+ protected static void WriteData(IBufferWriter<char> bufferWriter, int numChars)
+ {
+ Span<char> outputSpan = bufferWriter.GetSpan(numChars);
+ Debug.Assert(outputSpan.Length >= numChars);
+ var random = new Random(42);
Review Comment:
Need to remove static from this method and use the Random property from the base class for repeatable runs.
##########
src/Lucene.Net.Tests/Support/Analysis/TokenAttributes/TestCharTermAttributeImplBufferWriter.cs:
##########
@@ -0,0 +1,464 @@
+// Adapted from MIT-licensed ArrayBufferWriter tests here:
+// https://github.com/dotnet/runtime/blob/v10.0.6/src/libraries/System.Memory/tests/ArrayBufferWriter/ArrayBufferWriterTests.T.cs
+
+using Lucene.Net.Attributes;
+using Lucene.Net.Util;
+using NUnit.Framework;
+using System;
+using System.Buffers;
+using System.Collections.Generic;
+using System.Diagnostics;
+using System.Runtime.CompilerServices;
+using System.Runtime.InteropServices;
+
+namespace Lucene.Net.Analysis.TokenAttributes
+{
+ /*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+ /// <summary>
+ /// Tests for the <see cref="CharTermAttribute"/> class' <see cref="IBufferWriter{T}"/> implementation.
+ /// </summary>
+ [TestFixture]
+ [LuceneNetSpecific]
+ public class TestCharTermAttributeImplBufferWriter : LuceneTestCase
+ {
+ private const char DefaultChar = (char)0;
+
+ // LUCENENET specific: 10 is MIN_BUFFER_SIZE, but it gets oversized
+ private static readonly int DefaultBufferSize = ArrayUtil.Oversize(10, RamUsageEstimator.NUM_BYTES_CHAR);
+
+ // LUCENENET specific: works around a .NET Framework x64 RyuJIT bug where
+ // ReadOnlyMemory<char>.Span returns an empty span when the property is read
+ // inline on a local inside a large method (it only reproduces with the
+ // System.Memory polyfill; modern .NET is unaffected). Forcing the .Span read
+ // into its own non-inlined method makes the JIT compile it correctly. These
+ // helpers preserve the original assertion intent (comparing the Memory view
+ // against the Span view) on all target frameworks.
+ [MethodImpl(MethodImplOptions.NoInlining)]
+ private static bool SpanEqualsMemory(ReadOnlySpan<char> span, ReadOnlyMemory<char> memory)
+ => span.SequenceEqual(memory.Span);
+
+ [MethodImpl(MethodImplOptions.NoInlining)]
+ private static bool MemoryEqualsMemory(ReadOnlyMemory<char> a, ReadOnlyMemory<char> b)
+ => a.Span.SequenceEqual(b.Span);
+
+ [MethodImpl(MethodImplOptions.NoInlining)]
+ private static char MemoryElementAt(ReadOnlyMemory<char> memory, int index)
+ => memory.Span[index];
+
+ // LUCENENET specific: note that Clear behaves more like a Reset, which resets the position without clearing the
+ // buffer. so this test looks a lot more like the ResetWrittenCount in ArrayBufferWriter tests, than Clear.
+ [Test]
+ public void Clear()
+ {
+ var output = new CharTermAttribute();
+ int previousAvailable = output.FreeCapacity;
+ WriteData(output, 2);
+ Assert.True(output.FreeCapacity < previousAvailable);
+ Assert.True(output.Length > 0);
+ Assert.False(ReadOnlySpan<char>.Empty.SequenceEqual(output.AsSpan()));
+ Assert.False(MemoryEqualsMemory(ReadOnlyMemory<char>.Empty, output.AsMemory()));
+ Assert.True(SpanEqualsMemory(output.AsSpan(), output.AsMemory()));
+
+ ReadOnlyMemory<char> transientMemory = output.AsMemory();
+ ReadOnlySpan<char> transientSpan = output.AsSpan();
+ char t0 = MemoryElementAt(transientMemory, 0);
+ char t1 = transientSpan[1];
+ Assert.AreNotEqual(DefaultChar, t0);
+ Assert.AreNotEqual(DefaultChar, t1);
+ output.Clear();
+ Assert.AreEqual(t0, MemoryElementAt(transientMemory, 0));
+ Assert.AreEqual(t1, transientSpan[1]);
+
+ Assert.AreEqual(0, output.Length);
+ Assert.True(ReadOnlySpan<char>.Empty.SequenceEqual(output.AsSpan()));
+ Assert.True(MemoryEqualsMemory(ReadOnlyMemory<char>.Empty, output.AsMemory()));
+ Assert.AreEqual(previousAvailable, output.FreeCapacity);
+ }
+
+ // LUCENENET: this is equivalent to the ResetWrittenCount test
+ [Test]
+ public void SetLengthToZero()
+ {
+ var output = new CharTermAttribute();
+ int previousAvailable = output.FreeCapacity;
+ WriteData(output, 2);
+ Assert.True(output.FreeCapacity < previousAvailable);
+ Assert.True(output.Length > 0);
+ Assert.False(ReadOnlySpan<char>.Empty.SequenceEqual(output.AsSpan()));
+ Assert.False(MemoryEqualsMemory(ReadOnlyMemory<char>.Empty, output.AsMemory()));
+ Assert.True(SpanEqualsMemory(output.AsSpan(), output.AsMemory()));
+
+ ReadOnlyMemory<char> transientMemory = output.AsMemory();
+ ReadOnlySpan<char> transientSpan = output.AsSpan();
+ char t0 = MemoryElementAt(transientMemory, 0);
+ char t1 = transientSpan[1];
+ Assert.AreNotEqual(DefaultChar, t0);
+ Assert.AreNotEqual(DefaultChar, t1);
+ output.Length = 0;
+ Assert.AreEqual(t0, MemoryElementAt(transientMemory, 0));
+ Assert.AreEqual(t1, transientSpan[1]);
+
+ Assert.AreEqual(0, output.Length);
+ Assert.True(ReadOnlySpan<char>.Empty.SequenceEqual(output.AsSpan()));
+ Assert.True(MemoryEqualsMemory(ReadOnlyMemory<char>.Empty, output.AsMemory()));
+ Assert.AreEqual(previousAvailable, output.FreeCapacity);
+ }
+
+ [Test]
+ public void Advance()
+ {
+ {
+ var output = new CharTermAttribute();
+ int capacity = output.Capacity;
+ Assert.AreEqual(capacity, output.FreeCapacity);
+ output.Advance(output.FreeCapacity);
+ Assert.AreEqual(capacity, output.Length);
+ Assert.AreEqual(0, output.FreeCapacity);
+ }
+
+ {
+ var output = new CharTermAttribute();
+ output.Advance(output.Capacity);
+ Assert.AreEqual(output.Capacity, output.Length);
+ Assert.AreEqual(0, output.FreeCapacity);
+ int previousCapacity = output.Capacity;
+ Span<char> _ = output.GetSpan();
+ Assert.True(output.Capacity > previousCapacity);
+ }
+
+ {
+ var output = new CharTermAttribute();
+ WriteData(output, 2);
+ ReadOnlyMemory<char> previousMemory = output.AsMemory();
+ ReadOnlySpan<char> previousSpan = output.AsSpan();
+ Assert.True(SpanEqualsMemory(previousSpan, previousMemory));
+ output.Advance(10);
+ Assert.False(MemoryEqualsMemory(previousMemory, output.AsMemory()));
+ Assert.False(previousSpan.SequenceEqual(output.AsSpan()));
+ Assert.True(SpanEqualsMemory(output.AsSpan(), output.AsMemory()));
+ }
+
+ {
+ var output = new CharTermAttribute();
+ _ = output.GetSpan(20);
+ WriteData(output, 10);
+ ReadOnlyMemory<char> previousMemory = output.AsMemory();
+ ReadOnlySpan<char> previousSpan = output.AsSpan();
+ Assert.True(SpanEqualsMemory(previousSpan, previousMemory));
+ Assert.Throws<InvalidOperationException>(() => output.Advance(247));
+ output.Advance(10);
+ Assert.False(MemoryEqualsMemory(previousMemory, output.AsMemory()));
+ Assert.False(previousSpan.SequenceEqual(output.AsSpan()));
+ Assert.True(SpanEqualsMemory(output.AsSpan(), output.AsMemory()));
+ }
+ }
+
+ [Test]
+ public void AdvanceZero()
+ {
+ var output = new CharTermAttribute();
+ WriteData(output, 2);
+ Assert.AreEqual(2, output.Length);
+ ReadOnlyMemory<char> previousMemory = output.AsMemory();
+ ReadOnlySpan<char> previousSpan = output.AsSpan();
+ Assert.True(SpanEqualsMemory(previousSpan, previousMemory));
+ output.Advance(0);
+ Assert.AreEqual(2, output.Length);
+ Assert.True(MemoryEqualsMemory(previousMemory, output.AsMemory()));
+ Assert.True(previousSpan.SequenceEqual(output.AsSpan()));
+ Assert.True(SpanEqualsMemory(output.AsSpan(), output.AsMemory()));
+ }
+
+ [Test]
+ public void InvalidAdvance()
+ {
+ {
+ var output = new CharTermAttribute();
+ Assert.Throws<ArgumentOutOfRangeException>(() => output.Advance(-1));
+ Assert.Throws<InvalidOperationException>(() => output.Advance(output.Capacity + 1));
+ }
+
+ {
+ var output = new CharTermAttribute();
+ WriteData(output, 100);
+ Assert.Throws<InvalidOperationException>(() => output.Advance(output.FreeCapacity + 1));
+ }
+ }
+
+ [Test]
+ public void GetSpan_DefaultCtor()
+ {
+ var output = new CharTermAttribute();
+ Span<char> span = output.GetSpan();
+ Assert.AreEqual(DefaultBufferSize, span.Length);
+ }
+
+ [Test]
+ [TestCaseSource(nameof(SizeHints))]
+ public void GetSpan_DefaultCtor_WithSizeHint(int sizeHint)
+ {
+ var output = new CharTermAttribute();
+ Span<char> span = output.GetSpan(sizeHint);
+ // LUCENENET specific: due to our oversize logic, changed to GreaterOrEqual
+ Assert.GreaterOrEqual(span.Length, sizeHint <= DefaultBufferSize ? DefaultBufferSize : sizeHint);
+ }
+
+ [Test]
+ public void GetMemory_DefaultCtor()
+ {
+ var output = new CharTermAttribute();
+ Memory<char> memory = output.GetMemory();
+ Assert.AreEqual(DefaultBufferSize, memory.Length);
+ }
+
+ [Test]
+ [TestCaseSource(nameof(SizeHints))]
+ public void GetMemory_DefaultCtor_WithSizeHint(int sizeHint)
+ {
+ var output = new CharTermAttribute();
+ Memory<char> memory = output.GetMemory(sizeHint);
+ // LUCENENET specific: due to our oversize logic, changed to GreaterOrEqual
+ Assert.GreaterOrEqual(memory.Length, sizeHint <= DefaultBufferSize ? DefaultBufferSize : sizeHint);
+ }
+
+ [Test]
+ public void GetMemory_ExceedMaximumBufferSize_WithSmallStartingSize()
+ {
+ var output = new CharTermAttribute();
+ Assert.Throws<OutOfMemoryException>(() => output.GetMemory(int.MaxValue));
+ }
+
+ // LUCENENET specific: This test allocates a very large buffer to verify that Advance()
+ // performs its bounds check purely arithmetically (termLength vs. termBuffer.Length);
+ // Advance() itself never touches buffer memory, and WriteData only touches the first 1,000
+ // chars. If the allocation fails with OutOfMemoryException, the test is a no-op. Marked
+ // [Slow] due to the large allocation.
+ //
+ // NOTE: The upstream ArrayBufferWriter test gated this on Windows/macOS, and so do we. The
+ // risk is real on Linux but comes from the allocation itself, not from Advance(): under memory
+ // overcommit the kernel may grant the large buffer optimistically, then the GC's
+ // zero-initialization of the array touches every page, which can invoke the OOM killer (an
+ // uncatchable SIGKILL, not an OutOfMemoryException) and tear down the whole test run.
+ [Test]
+ [Slow]
+ public void InvalidAdvance_Large()
+ {
+ // LUCENENET specific: skip on Linux (see note above); the large allocation can be killed
+ // by the OOM killer with an uncatchable SIGKILL rather than a catchable OutOfMemoryException.
+ Assume.That(!Constants.LINUX, "Skipped on Linux: the large allocation may trigger the OOM killer.");
+
+ try
+ {
+ {
+ var output = new CharTermAttribute();
+
+ // LUCENENET specific: we don't (currently) have a ctor that takes a capacity,
+ // so we first request a span to force the buffer to grow to at least this size.
+ // The size stays under ~1.9B so it does not hit the ArrayUtil.Oversize int
+ // overflow clamp; the actual buffer is still oversized past the request.
+ // (The returned span is discarded; the grown buffer is retained by output.)
+ _ = output.GetSpan(1_500_000_000);
+
+ WriteData(output, 1_000);
+
+ // LUCENENET specific: the buffer is oversized past the requested size, so the
+ // over-advance boundary is Capacity-relative rather than a fixed request-based value.
+ Assert.Throws<InvalidOperationException>(() => output.Advance(int.MaxValue));
+ Assert.Throws<InvalidOperationException>(() => output.Advance(output.FreeCapacity + 1));
+ }
+ }
+ catch (OutOfMemoryException) { }
+ }
+
+ [Test]
+ public void GetMemoryAndSpan()
+ {
+ {
+ var output = new CharTermAttribute();
+ WriteData(output, 2);
+ Span<char> span = output.GetSpan();
+ Memory<char> memory = output.GetMemory();
+ Span<char> memorySpan = memory.Span;
+ Assert.True(span.Length > 0);
+ Assert.True(memorySpan.Length > 0);
+ Assert.AreEqual(span.Length, memorySpan.Length);
+ for (int i = 0; i < span.Length; i++)
+ {
+ Assert.AreEqual(DefaultChar, span[i]);
+ Assert.AreEqual(DefaultChar, memorySpan[i]);
+ }
+ }
+
+ {
+ var output = new CharTermAttribute();
+ WriteData(output, 2);
+ ReadOnlyMemory<char> writtenSoFarMemory = output.AsMemory();
+ ReadOnlySpan<char> writtenSoFar = output.AsSpan();
+ Assert.True(SpanEqualsMemory(writtenSoFar, writtenSoFarMemory));
+ int previousAvailable = output.FreeCapacity;
+ Span<char> span = output.GetSpan(500);
+ Assert.True(span.Length >= 500);
+ Assert.True(output.FreeCapacity >= 500);
+ Assert.True(output.FreeCapacity > previousAvailable);
+
+ Assert.AreEqual(writtenSoFar.Length, output.Length);
+ Assert.False(writtenSoFar.SequenceEqual(span.Slice(0, output.Length)));
+
+ Memory<char> memory = output.GetMemory();
+ Span<char> memorySpan = memory.Span;
+ Assert.True(span.Length >= 500);
+ Assert.True(memorySpan.Length >= 500);
+ Assert.AreEqual(span.Length, memorySpan.Length);
+ for (int i = 0; i < span.Length; i++)
+ {
+ Assert.AreEqual(DefaultChar, span[i]);
+ Assert.AreEqual(DefaultChar, memorySpan[i]);
+ }
+
+ memory = output.GetMemory(500);
+ memorySpan = memory.Span;
+ Assert.True(memorySpan.Length >= 500);
+ Assert.AreEqual(span.Length, memorySpan.Length);
+ for (int i = 0; i < memorySpan.Length; i++)
+ {
+ Assert.AreEqual(DefaultChar, memorySpan[i]);
+ }
+ }
+ }
+
+ [Test]
+ public void GetSpanShouldAtLeastDoubleWhenGrowing()
+ {
+ var output = new CharTermAttribute();
+ WriteData(output, 100);
+ int previousAvailable = output.FreeCapacity;
+
+ _ = output.GetSpan(previousAvailable);
+ Assert.AreEqual(previousAvailable, output.FreeCapacity);
+
+ _ = output.GetSpan(previousAvailable + 1);
+ Assert.True(output.FreeCapacity >= previousAvailable * 2);
+ }
+
+ [Test]
+ public void GetSpanOnlyGrowsAboveThreshold()
+ {
+ {
+ var output = new CharTermAttribute();
+ _ = output.GetSpan();
+ int previousAvailable = output.FreeCapacity;
+
+ for (int i = 0; i < 10; i++)
+ {
+ _ = output.GetSpan();
+ Assert.AreEqual(previousAvailable, output.FreeCapacity);
+ }
+ }
+
+ {
+ var output = new CharTermAttribute();
+ _ = output.GetSpan(10);
+ int previousAvailable = output.FreeCapacity;
+
+ for (int i = 0; i < 10; i++)
+ {
+ _ = output.GetSpan(previousAvailable);
+ Assert.AreEqual(previousAvailable, output.FreeCapacity);
+ }
+ }
+ }
+
+ [Test]
+ public void InvalidGetMemoryAndSpan()
+ {
+ var output = new CharTermAttribute();
+ WriteData(output, 2);
+ Assert.Throws<ArgumentOutOfRangeException>(() => output.GetSpan(-1));
+ Assert.Throws<ArgumentOutOfRangeException>(() => output.GetMemory(-1));
+ }
+
+ [Test]
+ public void MultipleCallsToGetSpan()
+ {
+ var output = new CharTermAttribute();
+ Assert.True(MemoryMarshal.TryGetArray(output.GetMemory(), out ArraySegment<char> array));
+ GCHandle pinnedArray = GCHandle.Alloc(array.Array, GCHandleType.Pinned);
+ try
+ {
+ int previousAvailable = output.FreeCapacity;
+ Assert.True(previousAvailable >= DefaultBufferSize);
+ Assert.True(output.Capacity >= DefaultBufferSize);
+ Assert.AreEqual(previousAvailable, output.Capacity);
+ Span<char> span = output.GetSpan();
+ Assert.True(span.Length >= previousAvailable);
+ Assert.True(span.Length >= DefaultBufferSize);
+ Span<char> newSpan = output.GetSpan();
+ Assert.AreEqual(span.Length, newSpan.Length);
+ // LUCENENET specific: changed expected to IntPtr.Zero due to presumed Xunit vs. NUnit differences
+ Assert.AreEqual(IntPtr.Zero,
+ Unsafe.ByteOffset(ref MemoryMarshal.GetReference(span), ref MemoryMarshal.GetReference(newSpan)));
+ Assert.AreEqual(span.Length, output.GetSpan().Length);
+ }
+ finally
+ {
+ pinnedArray.Free();
+ }
+ }
+
+ protected static void WriteData(IBufferWriter<char> bufferWriter, int numChars)
+ {
+ Span<char> outputSpan = bufferWriter.GetSpan(numChars);
+ Debug.Assert(outputSpan.Length >= numChars);
+ var random = new Random(42);
Review Comment:
See other comment about Random use.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@lucenenet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Implement IBufferWriter in OpenStringBuilder and CharTermAttribute, #1315 [lucenenet]
Posted by "paulirwin (via GitHub)" <gi...@apache.org>.
paulirwin commented on code in PR #1339:
URL: https://github.com/apache/lucenenet/pull/1339#discussion_r3404509166
##########
src/Lucene.Net.Analysis.Common/Analysis/Util/OpenStringBuilder.cs:
##########
@@ -400,5 +416,72 @@ IAppendable IAppendable.Append(ICharSequence value, int startIndex, int count)
ISpanAppendable ISpanAppendable.Append(ReadOnlySpan<char> value) => Append(value);
#endregion IAppendable Members
+
+ #region IBufferWriter<char> members and support
+
+ // LUCENENET-specific: IBufferWriter<char> support.
+ // See ArrayBufferWriter for an inspiration and reference implementation:
+ // https://github.com/dotnet/runtime/blob/v10.0.6/src/libraries/Common/src/System/Buffers/ArrayBufferWriter.cs
+
+ public void Advance(int count)
+ {
+ if (count < 0)
+ throw new ArgumentException(null, nameof(count));
+
+ if (m_len > m_buf.Length - count)
+ throw new InvalidOperationException($"Cannot advance {count} characters beyond the end of the buffer.");
+
+ m_len += count;
+ }
+
+ public Memory<char> GetMemory(int sizeHint = 0)
+ {
+ CheckAndResizeBufferForBufferWriter(sizeHint);
+ Debug.Assert(m_buf.Length > m_len);
+ return m_buf.AsMemory(m_len);
+ }
+
+ public Span<char> GetSpan(int sizeHint = 0)
+ {
+ CheckAndResizeBufferForBufferWriter(sizeHint);
+ Debug.Assert(m_buf.Length > m_len);
+ return m_buf.AsSpan(m_len);
+ }
+
+ private void CheckAndResizeBufferForBufferWriter(int sizeHint)
+ {
+ if (sizeHint < 0)
+ {
+ throw new ArgumentException("sizeHint must be non-negative", nameof(sizeHint));
+ }
+
+ if (sizeHint == 0)
+ {
+ sizeHint = 1;
Review Comment:
FYI - this behavior is what ArrayBufferWriter does: https://github.com/dotnet/runtime/blob/7706f546bac1a99b3d891afe3591dc88c67f0cc4/src/libraries/Common/src/System/Buffers/ArrayBufferWriter.cs#L206
Note that the only time you actually would get a span of length 1 (when `sizeHint == 0` or 1) is if there is only 1 char free capacity. If you don't pass in a size hint and the buffer has already grown greater than 1 (say, it has 500 chars free capacity) then you'll get back a span of length equal to FreeCapacity (i.e. 500), not 1, even though sizeHint is set to 1 in this branch. The 1-length case is probably pretty rare; and in that case, the caller has already said that they are fine if they get a span of length 1 by not providing a size hint.
Also note that if there is no more free capacity, this sizeHint of 1 will result in a doubling of the buffer, and returning a span of the prior buffer length (because it was doubled).
Example typical flow:
1. OpenStringBuilder is created with default capacity of 32
2. `GetSpan()` called with no sizeHint, returns a span of length 32
3. `Advance(24)` called to note 24 chars written
4. `GetSpan()` called again with no sizeHint, returns a span of length 8 (32 - 24)
5. Caller can tolerate an 8-char buffer because they gave no size hint. `Advance(8)` called to max out the written chars.
6. `GetSpan()` called again with no sizeHint, now there is no free capacity.
7. Internally, `EnsureCapacity` called with a sizeHint fallback of 1. 32 + 1 > 32, so `Resize(33)` is called.
8. Internally, `m_buf.Length << 1` (doubled) results in 64 which is bigger than 33, so the buffer resizes to 64.
9. Back to `GetSpan()`, it now returns a buffer of size 32 (64 capacity - 32 written)
Note that you can construct an example flow with a real span of size 1 returned above by replacing the number 24 with 31 in step 3, and 8 with 1 in steps 4-5. Then step 4 would return a span of length 1, but the caller has already said they don't care by not providing an argument.
In my opinion, we don't need to change this, as it already is memory-optimized to not unnecessarily increase the buffer size if the caller can tolerate short spans of the remaining capacity and doesn't care about what minimum size span they get back. Going to resolve this one, but feel free to reopen if you think there's a good alternative justification.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@lucenenet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Implement IBufferWriter in OpenStringBuilder and CharTermAttribute, #1315 [lucenenet]
Posted by "paulirwin (via GitHub)" <gi...@apache.org>.
paulirwin commented on code in PR #1339:
URL: https://github.com/apache/lucenenet/pull/1339#discussion_r3459980174
##########
src/Lucene.Net.Analysis.Common/Analysis/Util/OpenStringBuilder.cs:
##########
@@ -400,5 +416,72 @@ IAppendable IAppendable.Append(ICharSequence value, int startIndex, int count)
ISpanAppendable ISpanAppendable.Append(ReadOnlySpan<char> value) => Append(value);
#endregion IAppendable Members
+
+ #region IBufferWriter<char> members and support
+
+ // LUCENENET-specific: IBufferWriter<char> support.
+ // See ArrayBufferWriter for an inspiration and reference implementation:
+ // https://github.com/dotnet/runtime/blob/v10.0.6/src/libraries/Common/src/System/Buffers/ArrayBufferWriter.cs
+
+ public void Advance(int count)
+ {
+ if (count < 0)
+ throw new ArgumentException(null, nameof(count));
+
+ if (m_len > m_buf.Length - count)
+ throw new InvalidOperationException($"Cannot advance {count} characters beyond the end of the buffer.");
+
+ m_len += count;
+ }
+
+ public Memory<char> GetMemory(int sizeHint = 0)
+ {
+ CheckAndResizeBufferForBufferWriter(sizeHint);
+ Debug.Assert(m_buf.Length > m_len);
+ return m_buf.AsMemory(m_len);
+ }
+
+ public Span<char> GetSpan(int sizeHint = 0)
+ {
+ CheckAndResizeBufferForBufferWriter(sizeHint);
+ Debug.Assert(m_buf.Length > m_len);
+ return m_buf.AsSpan(m_len);
+ }
+
+ private void CheckAndResizeBufferForBufferWriter(int sizeHint)
+ {
+ if (sizeHint < 0)
+ {
+ throw new ArgumentException("sizeHint must be non-negative", nameof(sizeHint));
+ }
+
+ if (sizeHint == 0)
+ {
+ sizeHint = 1;
Review Comment:
I updated the minimums, even though we're talking about an edge case.
> It is reasonable to expect the "don't care" size to at least hold an average word.
Wanted to respond to this to say, it typically would in almost all cases even if the sizeHint fallback was 1, because the usual behavior would be to get a buffer of length 32 (when new), some arbitrary length less than 32 (if some small string has already been written), or a doubled-length buffer. Again, the only case where you would get one of length 1 would be if you had already written exactly 31 characters, or whatever `length - 1` is after it has doubled. But, I adjusted the minimums anyways.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@lucenenet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Implement IBufferWriter in OpenStringBuilder and CharTermAttribute, #1315 [lucenenet]
Posted by "paulirwin (via GitHub)" <gi...@apache.org>.
paulirwin merged PR #1339:
URL: https://github.com/apache/lucenenet/pull/1339
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@lucenenet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Implement IBufferWriter in OpenStringBuilder and CharTermAttribute, #1315 [lucenenet]
Posted by "paulirwin (via GitHub)" <gi...@apache.org>.
paulirwin commented on PR #1339:
URL: https://github.com/apache/lucenenet/pull/1339#issuecomment-4693205355
FYI - the failing tests on .NET Framework were surprisingly due to a JIT bug. I extracted a minimal reproduction and submitted the bug report to Microsoft. I worked around them by adding NoInlining methods around those calls which avoids the bug.
Feedback addressed or responded to; ready for re-review.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@lucenenet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Implement IBufferWriter in OpenStringBuilder and CharTermAttribute, #1315 [lucenenet]
Posted by "NightOwl888 (via GitHub)" <gi...@apache.org>.
NightOwl888 commented on code in PR #1339:
URL: https://github.com/apache/lucenenet/pull/1339#discussion_r3456194615
##########
src/Lucene.Net.Analysis.Common/Analysis/Util/OpenStringBuilder.cs:
##########
@@ -400,5 +416,72 @@ IAppendable IAppendable.Append(ICharSequence value, int startIndex, int count)
ISpanAppendable ISpanAppendable.Append(ReadOnlySpan<char> value) => Append(value);
#endregion IAppendable Members
+
+ #region IBufferWriter<char> members and support
+
+ // LUCENENET-specific: IBufferWriter<char> support.
+ // See ArrayBufferWriter for an inspiration and reference implementation:
+ // https://github.com/dotnet/runtime/blob/v10.0.6/src/libraries/Common/src/System/Buffers/ArrayBufferWriter.cs
+
+ public void Advance(int count)
+ {
+ if (count < 0)
+ throw new ArgumentException(null, nameof(count));
+
+ if (m_len > m_buf.Length - count)
+ throw new InvalidOperationException($"Cannot advance {count} characters beyond the end of the buffer.");
+
+ m_len += count;
+ }
+
+ public Memory<char> GetMemory(int sizeHint = 0)
+ {
+ CheckAndResizeBufferForBufferWriter(sizeHint);
+ Debug.Assert(m_buf.Length > m_len);
+ return m_buf.AsMemory(m_len);
+ }
+
+ public Span<char> GetSpan(int sizeHint = 0)
+ {
+ CheckAndResizeBufferForBufferWriter(sizeHint);
+ Debug.Assert(m_buf.Length > m_len);
+ return m_buf.AsSpan(m_len);
+ }
+
+ private void CheckAndResizeBufferForBufferWriter(int sizeHint)
+ {
+ if (sizeHint < 0)
+ {
+ throw new ArgumentException("sizeHint must be non-negative", nameof(sizeHint));
+ }
+
+ if (sizeHint == 0)
+ {
+ sizeHint = 1;
Review Comment:
Respectfully, I disagree. The `ArrayBufferWriter<T>` is a general-purpose buffer. `CharTermAttribute` and `OpenStringBuilder` are specifically designed to buffer text. It is reasonable to expect the "don't care" size to at least hold an average word. 1 `char` isn't even enough space to hold every character, since some of them require 2 chars.
`CharTermAttribute` uses a conservative value of 10 for the default size in the constructor. This seems like a good default size for that case. For `OpenStringBuilder`, I would probably choose 16 to mirror the `StringBuilder` constructors of both .NET and Java, even if this implementation uses a more aggressive 32.
Thanks for pointing out that the value returned is typically the whole remaining capacity. I had to double-check `MutableTextBuffer` is doing the same. Note that we may end up either replacing this implementation everywhere with `TextBuilder` or changing it to be a facade around the `MutableTextBuffer` implementation, anyway.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@lucenenet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Implement IBufferWriter in OpenStringBuilder and CharTermAttribute, #1315 [lucenenet]
Posted by "NightOwl888 (via GitHub)" <gi...@apache.org>.
NightOwl888 commented on code in PR #1339:
URL: https://github.com/apache/lucenenet/pull/1339#discussion_r3456215605
##########
src/Lucene.Net.Tests/Support/Analysis/TokenAttributes/TestCharTermAttributeImplBufferWriter.cs:
##########
@@ -0,0 +1,463 @@
+// Adapted from MIT-licensed ArrayBufferWriter tests here:
+// https://github.com/dotnet/runtime/blob/v10.0.6/src/libraries/System.Memory/tests/ArrayBufferWriter/ArrayBufferWriterTests.T.cs
+
+using Lucene.Net.Attributes;
+using Lucene.Net.Util;
+using NUnit.Framework;
+using System;
+using System.Buffers;
+using System.Collections.Generic;
+using System.Diagnostics;
+using System.Runtime.CompilerServices;
+using System.Runtime.InteropServices;
+
+namespace Lucene.Net.Analysis.TokenAttributes
+{
+ /*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+ /// <summary>
+ /// Tests for the <see cref="CharTermAttribute"/> class' <see cref="IBufferWriter{T}"/> implementation.
+ /// </summary>
+ [TestFixture]
+ [LuceneNetSpecific]
+ public class TestCharTermAttributeImplBufferWriter : LuceneTestCase
+ {
+ private const char DefaultChar = (char)0;
+
+ // LUCENENET specific: 10 is MIN_BUFFER_SIZE, but it gets oversized
+ private static readonly int DefaultBufferSize = ArrayUtil.Oversize(10, RamUsageEstimator.NUM_BYTES_CHAR);
+
+ // LUCENENET specific: works around a .NET Framework x64 RyuJIT bug where
+ // ReadOnlyMemory<char>.Span returns an empty span when the property is read
+ // inline on a local inside a large method (it only reproduces with the
+ // System.Memory polyfill; modern .NET is unaffected). Forcing the .Span read
+ // into its own non-inlined method makes the JIT compile it correctly. These
+ // helpers preserve the original assertion intent (comparing the Memory view
+ // against the Span view) on all target frameworks.
+ [MethodImpl(MethodImplOptions.NoInlining)]
+ private static bool SpanEqualsMemory(ReadOnlySpan<char> span, ReadOnlyMemory<char> memory)
+ => span.SequenceEqual(memory.Span);
+
+ [MethodImpl(MethodImplOptions.NoInlining)]
+ private static bool MemoryEqualsMemory(ReadOnlyMemory<char> a, ReadOnlyMemory<char> b)
+ => a.Span.SequenceEqual(b.Span);
+
+ [MethodImpl(MethodImplOptions.NoInlining)]
+ private static char MemoryElementAt(ReadOnlyMemory<char> memory, int index)
+ => memory.Span[index];
+
+ // LUCENENET specific: note that Clear behaves more like a Reset, which resets the position without clearing the
+ // buffer. so this test looks a lot more like the ResetWrittenCount in ArrayBufferWriter tests, than Clear.
+ [Test]
+ public void Clear()
+ {
+ var output = new CharTermAttribute();
+ int previousAvailable = output.FreeCapacity;
+ WriteData(output, 2);
+ Assert.True(output.FreeCapacity < previousAvailable);
+ Assert.True(output.Length > 0);
+ Assert.False(ReadOnlySpan<char>.Empty.SequenceEqual(output.AsSpan()));
+ Assert.False(MemoryEqualsMemory(ReadOnlyMemory<char>.Empty, output.AsMemory()));
+ Assert.True(SpanEqualsMemory(output.AsSpan(), output.AsMemory()));
+
+ ReadOnlyMemory<char> transientMemory = output.AsMemory();
+ ReadOnlySpan<char> transientSpan = output.AsSpan();
+ char t0 = MemoryElementAt(transientMemory, 0);
+ char t1 = transientSpan[1];
+ Assert.AreNotEqual(DefaultChar, t0);
+ Assert.AreNotEqual(DefaultChar, t1);
+ output.Clear();
+ Assert.AreEqual(t0, MemoryElementAt(transientMemory, 0));
+ Assert.AreEqual(t1, transientSpan[1]);
+
+ Assert.AreEqual(0, output.Length);
+ Assert.True(ReadOnlySpan<char>.Empty.SequenceEqual(output.AsSpan()));
+ Assert.True(MemoryEqualsMemory(ReadOnlyMemory<char>.Empty, output.AsMemory()));
+ Assert.AreEqual(previousAvailable, output.FreeCapacity);
+ }
+
+ // LUCENENET: this is equivalent to the ResetWrittenCount test
+ [Test]
+ public void SetLengthToZero()
+ {
+ var output = new CharTermAttribute();
+ int previousAvailable = output.FreeCapacity;
+ WriteData(output, 2);
+ Assert.True(output.FreeCapacity < previousAvailable);
+ Assert.True(output.Length > 0);
+ Assert.False(ReadOnlySpan<char>.Empty.SequenceEqual(output.AsSpan()));
+ Assert.False(MemoryEqualsMemory(ReadOnlyMemory<char>.Empty, output.AsMemory()));
+ Assert.True(SpanEqualsMemory(output.AsSpan(), output.AsMemory()));
+
+ ReadOnlyMemory<char> transientMemory = output.AsMemory();
+ ReadOnlySpan<char> transientSpan = output.AsSpan();
+ char t0 = MemoryElementAt(transientMemory, 0);
+ char t1 = transientSpan[1];
+ Assert.AreNotEqual(DefaultChar, t0);
+ Assert.AreNotEqual(DefaultChar, t1);
+ output.Length = 0;
+ Assert.AreEqual(t0, MemoryElementAt(transientMemory, 0));
+ Assert.AreEqual(t1, transientSpan[1]);
+
+ Assert.AreEqual(0, output.Length);
+ Assert.True(ReadOnlySpan<char>.Empty.SequenceEqual(output.AsSpan()));
+ Assert.True(MemoryEqualsMemory(ReadOnlyMemory<char>.Empty, output.AsMemory()));
+ Assert.AreEqual(previousAvailable, output.FreeCapacity);
+ }
+
+ [Test]
+ public void Advance()
+ {
+ {
+ var output = new CharTermAttribute();
+ int capacity = output.Capacity;
+ Assert.AreEqual(capacity, output.FreeCapacity);
+ output.Advance(output.FreeCapacity);
+ Assert.AreEqual(capacity, output.Length);
+ Assert.AreEqual(0, output.FreeCapacity);
+ }
+
+ {
+ var output = new CharTermAttribute();
+ output.Advance(output.Capacity);
+ Assert.AreEqual(output.Capacity, output.Length);
+ Assert.AreEqual(0, output.FreeCapacity);
+ int previousCapacity = output.Capacity;
+ Span<char> _ = output.GetSpan();
+ Assert.True(output.Capacity > previousCapacity);
+ }
+
+ {
+ var output = new CharTermAttribute();
+ WriteData(output, 2);
+ ReadOnlyMemory<char> previousMemory = output.AsMemory();
+ ReadOnlySpan<char> previousSpan = output.AsSpan();
+ Assert.True(SpanEqualsMemory(previousSpan, previousMemory));
+ output.Advance(10);
+ Assert.False(MemoryEqualsMemory(previousMemory, output.AsMemory()));
+ Assert.False(previousSpan.SequenceEqual(output.AsSpan()));
+ Assert.True(SpanEqualsMemory(output.AsSpan(), output.AsMemory()));
+ }
+
+ {
+ var output = new CharTermAttribute();
+ _ = output.GetSpan(20);
+ WriteData(output, 10);
+ ReadOnlyMemory<char> previousMemory = output.AsMemory();
+ ReadOnlySpan<char> previousSpan = output.AsSpan();
+ Assert.True(SpanEqualsMemory(previousSpan, previousMemory));
+ Assert.Throws<InvalidOperationException>(() => output.Advance(247));
+ output.Advance(10);
+ Assert.False(MemoryEqualsMemory(previousMemory, output.AsMemory()));
+ Assert.False(previousSpan.SequenceEqual(output.AsSpan()));
+ Assert.True(SpanEqualsMemory(output.AsSpan(), output.AsMemory()));
+ }
+ }
+
+ [Test]
+ public void AdvanceZero()
+ {
+ var output = new CharTermAttribute();
+ WriteData(output, 2);
+ Assert.AreEqual(2, output.Length);
+ ReadOnlyMemory<char> previousMemory = output.AsMemory();
+ ReadOnlySpan<char> previousSpan = output.AsSpan();
+ Assert.True(SpanEqualsMemory(previousSpan, previousMemory));
+ output.Advance(0);
+ Assert.AreEqual(2, output.Length);
+ Assert.True(MemoryEqualsMemory(previousMemory, output.AsMemory()));
+ Assert.True(previousSpan.SequenceEqual(output.AsSpan()));
+ Assert.True(SpanEqualsMemory(output.AsSpan(), output.AsMemory()));
+ }
+
+ [Test]
+ public void InvalidAdvance()
+ {
+ {
+ var output = new CharTermAttribute();
+ Assert.Throws<ArgumentOutOfRangeException>(() => output.Advance(-1));
+ Assert.Throws<InvalidOperationException>(() => output.Advance(output.Capacity + 1));
+ }
+
+ {
+ var output = new CharTermAttribute();
+ WriteData(output, 100);
+ Assert.Throws<InvalidOperationException>(() => output.Advance(output.FreeCapacity + 1));
+ }
+ }
+
+ [Test]
+ public void GetSpan_DefaultCtor()
+ {
+ var output = new CharTermAttribute();
+ Span<char> span = output.GetSpan();
+ Assert.AreEqual(DefaultBufferSize, span.Length);
+ }
+
+ [Test]
+ [TestCaseSource(nameof(SizeHints))]
+ public void GetSpan_DefaultCtor_WithSizeHint(int sizeHint)
+ {
+ var output = new CharTermAttribute();
+ Span<char> span = output.GetSpan(sizeHint);
+ // LUCENENET specific: due to our oversize logic, changed to GreaterOrEqual
+ Assert.GreaterOrEqual(span.Length, sizeHint <= DefaultBufferSize ? DefaultBufferSize : sizeHint);
+ }
+
+ [Test]
+ public void GetMemory_DefaultCtor()
+ {
+ var output = new CharTermAttribute();
+ Memory<char> memory = output.GetMemory();
+ Assert.AreEqual(DefaultBufferSize, memory.Length);
+ }
+
+ [Test]
+ [TestCaseSource(nameof(SizeHints))]
+ public void GetMemory_DefaultCtor_WithSizeHint(int sizeHint)
+ {
+ var output = new CharTermAttribute();
+ Memory<char> memory = output.GetMemory(sizeHint);
+ // LUCENENET specific: due to our oversize logic, changed to GreaterOrEqual
+ Assert.GreaterOrEqual(memory.Length, sizeHint <= DefaultBufferSize ? DefaultBufferSize : sizeHint);
+ }
+
+ [Test]
+ public void GetMemory_ExceedMaximumBufferSize_WithSmallStartingSize()
+ {
+ var output = new CharTermAttribute();
+ Assert.Throws<OutOfMemoryException>(() => output.GetMemory(int.MaxValue));
+ }
+
+ // LUCENENET specific: This test allocates a very large buffer to verify that Advance()
+ // performs its bounds check purely arithmetically (termLength vs. termBuffer.Length);
+ // Advance() itself never touches buffer memory, and WriteData only touches the first 1,000
+ // chars. If the allocation fails with OutOfMemoryException, the test is a no-op. Marked
+ // [Slow] due to the large allocation.
+ //
+ // NOTE: The upstream ArrayBufferWriter test gated this on Windows/macOS, and so do we. The
+ // risk is real on Linux but comes from the allocation itself, not from Advance(): under memory
+ // overcommit the kernel may grant the large buffer optimistically, then the GC's
+ // zero-initialization of the array touches every page, which can invoke the OOM killer (an
+ // uncatchable SIGKILL, not an OutOfMemoryException) and tear down the whole test run.
+ [Test]
+ [Slow]
Review Comment:
We should probably make this `[Nightly]` rather than `[Slow]`.
##########
src/Lucene.Net.Tests.Analysis.Common/Support/Analysis/Util/TestOpenStringBuilderBufferWriter.cs:
##########
@@ -0,0 +1,506 @@
+// Adapted from MIT-licensed ArrayBufferWriter tests here:
+// https://github.com/dotnet/runtime/blob/v10.0.6/src/libraries/System.Memory/tests/ArrayBufferWriter/ArrayBufferWriterTests.T.cs
+
+using Lucene.Net.Attributes;
+using Lucene.Net.Util;
+using NUnit.Framework;
+using System;
+using System.Buffers;
+using System.Collections.Generic;
+using System.Diagnostics;
+using System.Runtime.CompilerServices;
+using System.Runtime.InteropServices;
+
+namespace Lucene.Net.Analysis.Util
+{
+ /*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+ /// <summary>
+ /// Tests for <see cref="OpenStringBuilder"/>'s <see cref="IBufferWriter{T}"/> implementation.
+ /// </summary>
+ [TestFixture]
+ [LuceneNetSpecific]
+ public class TestOpenStringBuilderBufferWriter : LuceneTestCase
+ {
+ private const char DefaultChar = (char)0;
+ private const int DefaultBufferSize = 32; // copied from hardcoded value in OpenStringBuilder
+
+ // LUCENENET specific: works around a .NET Framework x64 RyuJIT bug where
+ // ReadOnlyMemory<char>.Span returns an empty span when the property is read
+ // inline on a local inside a large method (it only reproduces with the
+ // System.Memory polyfill; modern .NET is unaffected). Forcing the .Span read
+ // into its own non-inlined method makes the JIT compile it correctly. These
+ // helpers preserve the original assertion intent (comparing the Memory view
+ // against the Span view) on all target frameworks.
+ [MethodImpl(MethodImplOptions.NoInlining)]
+ private static bool SpanEqualsMemory(ReadOnlySpan<char> span, ReadOnlyMemory<char> memory)
+ => span.SequenceEqual(memory.Span);
+
+ [MethodImpl(MethodImplOptions.NoInlining)]
+ private static bool MemoryEqualsMemory(ReadOnlyMemory<char> a, ReadOnlyMemory<char> b)
+ => a.Span.SequenceEqual(b.Span);
+
+ [MethodImpl(MethodImplOptions.NoInlining)]
+ private static char MemoryElementAt(ReadOnlyMemory<char> memory, int index)
+ => memory.Span[index];
+
+ [Test]
+ public void ArrayBufferWriter_Ctor()
+ {
+ {
+ var output = new OpenStringBuilder();
+ Assert.AreEqual(DefaultBufferSize, output.FreeCapacity);
+ Assert.AreEqual(DefaultBufferSize, output.Capacity);
+ Assert.AreEqual(0, output.Length);
+ Assert.True(ReadOnlySpan<char>.Empty.SequenceEqual(output.AsSpan()));
+ Assert.True(MemoryEqualsMemory(ReadOnlyMemory<char>.Empty, output.AsMemory()));
+ }
+
+ {
+ var output = new OpenStringBuilder(200);
+ Assert.True(output.FreeCapacity >= 200);
+ Assert.True(output.Capacity >= 200);
+ Assert.AreEqual(0, output.Length);
+ Assert.True(ReadOnlySpan<char>.Empty.SequenceEqual(output.AsSpan()));
+ Assert.True(MemoryEqualsMemory(ReadOnlyMemory<char>.Empty, output.AsMemory()));
+ }
+
+ {
+ OpenStringBuilder output = default;
+ Assert.Null(output);
+ }
+ }
+
+ [Test]
+ public void Invalid_Ctor()
+ {
+ // LUCENENET specific - changed to ArgumentOutOfRangeException
+ Assert.Throws<ArgumentOutOfRangeException>(() => new OpenStringBuilder(0));
+ Assert.Throws<ArgumentOutOfRangeException>(() => new OpenStringBuilder(-1));
+ Assert.Throws<OutOfMemoryException>(() => new OpenStringBuilder(int.MaxValue));
+ }
+
+ [Test]
+ public void Reset()
+ {
+ var output = new OpenStringBuilder(256);
+ int previousAvailable = output.FreeCapacity;
+ WriteData(output, 2);
+ Assert.True(output.FreeCapacity < previousAvailable);
+ Assert.True(output.Length > 0);
+ Assert.False(ReadOnlySpan<char>.Empty.SequenceEqual(output.AsSpan()));
+ Assert.False(MemoryEqualsMemory(ReadOnlyMemory<char>.Empty, output.AsMemory()));
+ Assert.True(SpanEqualsMemory(output.AsSpan(), output.AsMemory()));
+
+ ReadOnlyMemory<char> transientMemory = output.AsMemory();
+ ReadOnlySpan<char> transientSpan = output.AsSpan();
+ char t0 = MemoryElementAt(transientMemory, 0);
+ char t1 = transientSpan[1];
+ Assert.AreNotEqual(DefaultChar, t0);
+ Assert.AreNotEqual(DefaultChar, t1);
+ output.Reset();
+ Assert.AreEqual(t0, MemoryElementAt(transientMemory, 0));
+ Assert.AreEqual(t1, transientSpan[1]);
+
+ Assert.AreEqual(0, output.Length);
+ Assert.True(ReadOnlySpan<char>.Empty.SequenceEqual(output.AsSpan()));
+ Assert.True(MemoryEqualsMemory(ReadOnlyMemory<char>.Empty, output.AsMemory()));
+ Assert.AreEqual(previousAvailable, output.FreeCapacity);
+ }
+
+ [Test]
+ public void Advance()
+ {
+ {
+ var output = new OpenStringBuilder();
+ int capacity = output.Capacity;
+ Assert.AreEqual(capacity, output.FreeCapacity);
+ output.Advance(output.FreeCapacity);
+ Assert.AreEqual(capacity, output.Length);
+ Assert.AreEqual(0, output.FreeCapacity);
+ }
+
+ {
+ var output = new OpenStringBuilder();
+ output.Advance(output.Capacity);
+ Assert.AreEqual(output.Capacity, output.Length);
+ Assert.AreEqual(0, output.FreeCapacity);
+ int previousCapacity = output.Capacity;
+ Span<char> _ = output.GetSpan();
+ Assert.True(output.Capacity > previousCapacity);
+ }
+
+ {
+ var output = new OpenStringBuilder(256);
+ WriteData(output, 2);
+ ReadOnlyMemory<char> previousMemory = output.AsMemory();
+ ReadOnlySpan<char> previousSpan = output.AsSpan();
+ Assert.True(SpanEqualsMemory(previousSpan, previousMemory));
+ output.Advance(10);
+ Assert.False(MemoryEqualsMemory(previousMemory, output.AsMemory()));
+ Assert.False(previousSpan.SequenceEqual(output.AsSpan()));
+ Assert.True(SpanEqualsMemory(output.AsSpan(), output.AsMemory()));
+ }
+
+ {
+ var output = new OpenStringBuilder();
+ _ = output.GetSpan(20);
+ WriteData(output, 10);
+ ReadOnlyMemory<char> previousMemory = output.AsMemory();
+ ReadOnlySpan<char> previousSpan = output.AsSpan();
+ Assert.True(SpanEqualsMemory(previousSpan, previousMemory));
+ Assert.Throws<InvalidOperationException>(() => output.Advance(247));
+ output.Advance(10);
+ Assert.False(MemoryEqualsMemory(previousMemory, output.AsMemory()));
+ Assert.False(previousSpan.SequenceEqual(output.AsSpan()));
+ Assert.True(SpanEqualsMemory(output.AsSpan(), output.AsMemory()));
+ }
+ }
+
+ [Test]
+ public void AdvanceZero()
+ {
+ var output = new OpenStringBuilder();
+ WriteData(output, 2);
+ Assert.AreEqual(2, output.Length);
+ ReadOnlyMemory<char> previousMemory = output.AsMemory();
+ ReadOnlySpan<char> previousSpan = output.AsSpan();
+ Assert.True(SpanEqualsMemory(previousSpan, previousMemory));
+ output.Advance(0);
+ Assert.AreEqual(2, output.Length);
+ Assert.True(MemoryEqualsMemory(previousMemory, output.AsMemory()));
+ Assert.True(previousSpan.SequenceEqual(output.AsSpan()));
+ Assert.True(SpanEqualsMemory(output.AsSpan(), output.AsMemory()));
+ }
+
+ [Test]
+ public void InvalidAdvance()
+ {
+ {
+ var output = new OpenStringBuilder();
+ Assert.Throws<ArgumentOutOfRangeException>(() => output.Advance(-1));
+ Assert.Throws<InvalidOperationException>(() => output.Advance(output.Capacity + 1));
+ }
+
+ {
+ var output = new OpenStringBuilder();
+ WriteData(output, 100);
+ Assert.Throws<InvalidOperationException>(() => output.Advance(output.FreeCapacity + 1));
+ }
+ }
+
+ [Test]
+ public void GetSpan_DefaultCtor()
+ {
+ var output = new OpenStringBuilder();
+ Span<char> span = output.GetSpan();
+ Assert.AreEqual(DefaultBufferSize, span.Length);
+ }
+
+ [Test]
+ [TestCaseSource(nameof(SizeHints))]
+ public void GetSpan_DefaultCtor_WithSizeHint(int sizeHint)
+ {
+ var output = new OpenStringBuilder();
+ Span<char> span = output.GetSpan(sizeHint);
+ Assert.AreEqual(sizeHint <= DefaultBufferSize ? DefaultBufferSize : sizeHint, span.Length);
+ }
+
+ [Test]
+ public void GetSpan_InitSizeCtor()
+ {
+ var output = new OpenStringBuilder(100);
+ Span<char> span = output.GetSpan();
+ Assert.AreEqual(100, span.Length);
+ }
+
+ [Test]
+ [TestCaseSource(nameof(SizeHints))]
+ public void GetSpan_InitSizeCtor_WithSizeHint(int sizeHint)
+ {
+ {
+ var output = new OpenStringBuilder(256);
+ Span<char> span = output.GetSpan(sizeHint);
+ // LUCENENET specific: due to our oversize logic, changed to GreaterOrEqual and removed sizeHint addend
+ Assert.GreaterOrEqual(span.Length, sizeHint <= 256 ? 256 : sizeHint);
+ }
+
+ {
+ var output = new OpenStringBuilder(1000);
+ Span<char> span = output.GetSpan(sizeHint);
+ // LUCENENET specific: due to our oversize logic, changed to GreaterOrEqual and removed sizeHint addend
+ Assert.GreaterOrEqual(span.Length, sizeHint <= 1000 ? 1000 : sizeHint);
+ }
+ }
+
+ [Test]
+ public void GetMemory_DefaultCtor()
+ {
+ var output = new OpenStringBuilder();
+ Memory<char> memory = output.GetMemory();
+ Assert.AreEqual(DefaultBufferSize, memory.Length);
+ }
+
+ [Test]
+ [TestCaseSource(nameof(SizeHints))]
+ public void GetMemory_DefaultCtor_WithSizeHint(int sizeHint)
+ {
+ var output = new OpenStringBuilder();
+ Memory<char> memory = output.GetMemory(sizeHint);
+ Assert.AreEqual(sizeHint <= DefaultBufferSize ? DefaultBufferSize : sizeHint, memory.Length);
+ }
+
+ [Test]
+ public void GetMemory_ExceedMaximumBufferSize_WithSmallStartingSize()
+ {
+ var output = new OpenStringBuilder(256);
+ Assert.Throws<OutOfMemoryException>(() => output.GetMemory(int.MaxValue));
+ }
+
+ [Test]
+ public void GetMemory_InitSizeCtor()
+ {
+ var output = new OpenStringBuilder(100);
+ Memory<char> memory = output.GetMemory();
+ Assert.AreEqual(100, memory.Length);
+ }
+
+ [Test]
+ [TestCaseSource(nameof(SizeHints))]
+ public void GetMemory_InitSizeCtor_WithSizeHint(int sizeHint)
+ {
+ {
+ var output = new OpenStringBuilder(256);
+ Memory<char> memory = output.GetMemory(sizeHint);
+ // LUCENENET specific: due to our oversize logic, changed to GreaterOrEqual and removed sizeHint addend
+ Assert.GreaterOrEqual(memory.Length, sizeHint <= 256 ? 256 : sizeHint);
+ }
+
+ {
+ var output = new OpenStringBuilder(1000);
+ Memory<char> memory = output.GetMemory(sizeHint);
+ // LUCENENET specific: due to our oversize logic, changed to GreaterOrEqual and removed sizeHint addend
+ Assert.GreaterOrEqual(memory.Length, sizeHint <= 1000 ? 1000 : sizeHint);
+ }
+ }
+
+ // LUCENENET specific: This test allocates a very large buffer to verify that Advance()
+ // performs its bounds check purely arithmetically (m_len vs. m_buf.Length); Advance() itself
+ // never touches buffer memory, and WriteData only touches the first 1,000 chars. If the
+ // allocation fails with OutOfMemoryException, the test is a no-op. Marked [Slow] due to the
+ // large allocation.
+ //
+ // NOTE: The upstream ArrayBufferWriter test gated this on Windows/macOS, and so do we. The
+ // risk is real on Linux but comes from the allocation itself, not from Advance(): under memory
+ // overcommit the kernel may grant new char[2_000_000_000] optimistically, then the GC's
+ // zero-initialization of the array touches every page, which can invoke the OOM killer (an
+ // uncatchable SIGKILL, not an OutOfMemoryException) and tear down the whole test run.
+ [Test]
+ [Slow]
Review Comment:
We should probably make this `[Nightly]` rather than `[Slow]`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@lucenenet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Implement IBufferWriter in OpenStringBuilder and CharTermAttribute, #1315 [lucenenet]
Posted by "paulirwin (via GitHub)" <gi...@apache.org>.
paulirwin commented on code in PR #1339:
URL: https://github.com/apache/lucenenet/pull/1339#discussion_r3404511260
##########
src/Lucene.Net/Analysis/TokenAttributes/CharTermAttributeImpl.cs:
##########
@@ -516,5 +529,121 @@ public override void CopyTo(IAttribute target) // LUCENENET specific - intention
ISpanAppendable ISpanAppendable.Append(ReadOnlySpan<char> value) => Append(value);
#endregion
+
+ #region IBufferWriter<char> members and implementation support
+
+ // LUCENENET-specific: IBufferWriter<char> support.
+ // See ArrayBufferWriter for an inspiration and reference implementation:
+ // https://github.com/dotnet/runtime/blob/v10.0.6/src/libraries/Common/src/System/Buffers/ArrayBufferWriter.cs
+
+ /// <summary>
+ /// Notifies this instance that <paramref name="count"/> characters were
+ /// written to the buffer returned by the most recent call to
+ /// <see cref="GetSpan(int)"/> or <see cref="GetMemory(int)"/>, advancing
+ /// the term length accordingly.
+ /// <para/>
+ /// LUCENENET specific - implements <see cref="System.Buffers.IBufferWriter{T}"/>.
+ /// </summary>
+ /// <param name="count">The number of characters written to the buffer.</param>
+ /// <exception cref="ArgumentException">
+ /// <paramref name="count"/> is negative.
+ /// </exception>
+ /// <exception cref="InvalidOperationException">
+ /// Advancing by <paramref name="count"/> would move past the end of the
+ /// available buffer space.
+ /// </exception>
+ /// <remarks>
+ /// You must request a new buffer after calling Advance to continue writing more data
+ /// and cannot write to a previously acquired buffer.
+ /// </remarks>
+ public void Advance(int count)
+ {
+ if (count < 0)
+ throw new ArgumentException(null, nameof(count));
+
+ if (termLength > termBuffer.Length - count)
+ throw new InvalidOperationException($"Cannot advance {count} characters beyond the end of the buffer.");
+
+ termLength += count;
+ }
+
+ /// <summary>
+ /// Returns a <see cref="Memory{T}"/> to write to, starting at the current
+ /// term length, that is at least <paramref name="sizeHint"/> characters in
+ /// length. The buffer is grown if necessary; the returned memory is never
+ /// empty.
+ /// <para/>
+ /// LUCENENET specific - implements <see cref="System.Buffers.IBufferWriter{T}"/>.
+ /// </summary>
+ /// <param name="sizeHint">
+ /// The minimum requested length of the returned <see cref="Memory{T}"/>.
+ /// A value of <c>0</c> requests a non-empty buffer of unspecified size.
+ /// </param>
+ /// <returns>
+ /// A <see cref="Memory{T}"/> of at least <paramref name="sizeHint"/>
+ /// characters (and at least one character).
+ /// </returns>
+ /// <exception cref="ArgumentException">
+ /// <paramref name="sizeHint"/> is negative.
+ /// </exception>
+ public Memory<char> GetMemory(int sizeHint = 0)
+ {
+ CheckAndResizeBufferForBufferWriter(sizeHint);
+ Debug.Assert(termBuffer.Length > termLength);
+ return termBuffer.AsMemory(termLength);
+ }
+
+ /// <summary>
+ /// Returns a <see cref="Span{T}"/> to write to, starting at the current
+ /// term length, that is at least <paramref name="sizeHint"/> characters in
+ /// length. The buffer is grown if necessary; the returned span is never
+ /// empty.
+ /// <para/>
+ /// LUCENENET specific - implements <see cref="System.Buffers.IBufferWriter{T}"/>.
+ /// </summary>
+ /// <param name="sizeHint">
+ /// The minimum requested length of the returned <see cref="Span{T}"/>.
+ /// A value of <c>0</c> requests a non-empty buffer of unspecified size.
+ /// </param>
+ /// <returns>
+ /// A <see cref="Span{T}"/> of at least <paramref name="sizeHint"/>
+ /// characters (and at least one character).
+ /// </returns>
+ /// <exception cref="ArgumentException">
+ /// <paramref name="sizeHint"/> is negative.
+ /// </exception>
+ public Span<char> GetSpan(int sizeHint = 0)
+ {
+ CheckAndResizeBufferForBufferWriter(sizeHint);
+ Debug.Assert(termBuffer.Length > termLength);
+ return termBuffer.AsSpan(termLength);
+ }
+
+ private void CheckAndResizeBufferForBufferWriter(int sizeHint)
+ {
+ if (sizeHint < 0)
+ {
+ throw new ArgumentException("sizeHint must be non-negative", nameof(sizeHint));
+ }
+
+ if (sizeHint == 0)
+ {
+ sizeHint = 1;
Review Comment:
See other comment.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@lucenenet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Implement IBufferWriter in OpenStringBuilder and CharTermAttribute, #1315 [lucenenet]
Posted by "NightOwl888 (via GitHub)" <gi...@apache.org>.
NightOwl888 commented on code in PR #1339:
URL: https://github.com/apache/lucenenet/pull/1339#discussion_r3399363623
##########
src/Lucene.Net.Analysis.Common/Analysis/Util/OpenStringBuilder.cs:
##########
@@ -400,5 +416,72 @@ IAppendable IAppendable.Append(ICharSequence value, int startIndex, int count)
ISpanAppendable ISpanAppendable.Append(ReadOnlySpan<char> value) => Append(value);
#endregion IAppendable Members
+
+ #region IBufferWriter<char> members and support
+
+ // LUCENENET-specific: IBufferWriter<char> support.
+ // See ArrayBufferWriter for an inspiration and reference implementation:
+ // https://github.com/dotnet/runtime/blob/v10.0.6/src/libraries/Common/src/System/Buffers/ArrayBufferWriter.cs
+
+ public void Advance(int count)
+ {
+ if (count < 0)
+ throw new ArgumentException(null, nameof(count));
+
+ if (m_len > m_buf.Length - count)
+ throw new InvalidOperationException($"Cannot advance {count} characters beyond the end of the buffer.");
+
+ m_len += count;
+ }
+
+ public Memory<char> GetMemory(int sizeHint = 0)
+ {
+ CheckAndResizeBufferForBufferWriter(sizeHint);
+ Debug.Assert(m_buf.Length > m_len);
+ return m_buf.AsMemory(m_len);
+ }
+
+ public Span<char> GetSpan(int sizeHint = 0)
+ {
+ CheckAndResizeBufferForBufferWriter(sizeHint);
+ Debug.Assert(m_buf.Length > m_len);
+ return m_buf.AsSpan(m_len);
+ }
+
+ private void CheckAndResizeBufferForBufferWriter(int sizeHint)
+ {
+ if (sizeHint < 0)
+ {
+ throw new ArgumentException("sizeHint must be non-negative", nameof(sizeHint));
+ }
+
+ if (sizeHint == 0)
+ {
+ sizeHint = 1;
Review Comment:
Since the default constructor uses an initial size of 32, we should probably provide something more reasonable here. Both the Java and .NET StringBuilder types have a default value of 16. That is generally enough to write most numbers or at least one long word.
##########
src/Lucene.Net.Analysis.Common/Analysis/Util/OpenStringBuilder.cs:
##########
@@ -400,5 +416,72 @@ IAppendable IAppendable.Append(ICharSequence value, int startIndex, int count)
ISpanAppendable ISpanAppendable.Append(ReadOnlySpan<char> value) => Append(value);
#endregion IAppendable Members
+
+ #region IBufferWriter<char> members and support
+
+ // LUCENENET-specific: IBufferWriter<char> support.
+ // See ArrayBufferWriter for an inspiration and reference implementation:
+ // https://github.com/dotnet/runtime/blob/v10.0.6/src/libraries/Common/src/System/Buffers/ArrayBufferWriter.cs
+
+ public void Advance(int count)
+ {
+ if (count < 0)
+ throw new ArgumentException(null, nameof(count));
+
+ if (m_len > m_buf.Length - count)
+ throw new InvalidOperationException($"Cannot advance {count} characters beyond the end of the buffer.");
+
+ m_len += count;
+ }
+
+ public Memory<char> GetMemory(int sizeHint = 0)
+ {
+ CheckAndResizeBufferForBufferWriter(sizeHint);
+ Debug.Assert(m_buf.Length > m_len);
+ return m_buf.AsMemory(m_len);
+ }
+
+ public Span<char> GetSpan(int sizeHint = 0)
+ {
+ CheckAndResizeBufferForBufferWriter(sizeHint);
+ Debug.Assert(m_buf.Length > m_len);
+ return m_buf.AsSpan(m_len);
+ }
+
+ private void CheckAndResizeBufferForBufferWriter(int sizeHint)
+ {
+ if (sizeHint < 0)
+ {
+ throw new ArgumentException("sizeHint must be non-negative", nameof(sizeHint));
+ }
+
+ if (sizeHint == 0)
+ {
+ sizeHint = 1;
+ }
+
+ EnsureCapacity(sizeHint);
+ }
+
+ /// <summary>
+ /// Returns the amount of space available that can still be written into without forcing the underlying buffer to grow.
+ /// </summary>
+ public int FreeCapacity => m_buf.Length - m_len;
+
+ #endregion
+
+ #region AsSpan/AsMemory support
+
+ /// <summary>
+ /// Returns the data written to the underlying buffer so far, as a <see cref="ReadOnlyMemory{T}"/>.
+ /// </summary>
+ public ReadOnlyMemory<char> AsMemory() => m_buf.AsMemory(0, m_len);
+
+ /// <summary>
+ /// Returns the data written to the underlying buffer so far, as a <see cref="ReadOnlySpan{T}"/>.
+ /// </summary>
+ public ReadOnlySpan<char> AsSpan() => m_buf.AsSpan(0, m_len);
Review Comment:
One thing I discovered while designing `TextBuilder` is that it is more sensible to implement `AsSpan()` as extension methods on reference types. Why? Because reference types can be `null` and `null` naturally converts to `ReadOnlySpan<char>.Empty`. If we implement it here, it will throw a `NullReferenceException` when the value is `null`
Another thing to note is that it would be most sensible to put these extension methods into the same namespace `Lucene.Net.Analysis.Util`. In this case, I would suggest simply dropping an `OpenStringBuilderExtensions` class right in this file.
Note also that there are 6 highly recommended overloads of these methods:
`public static ReadOnlySpan<char> AsSpan(this OpenStringBuilder? text)`
`public static ReadOnlySpan<char> AsSpan(this OpenStringBuilder? text, int start)`
`public static ReadOnlySpan<char> AsSpan(this OpenStringBuilder? text, int start, int length)`
`public static ReadOnlyMemory<char> AsMemory(this OpenStringBuilder? text)`
`public static ReadOnlyMemory<char> AsMemory(this OpenStringBuilder? text, int start)`
`public static ReadOnlyMemory<char> AsMemory(this OpenStringBuilder? text, int start, int length)`
There are also 4 additional overloads that are a nice finishing touch:
`public static ReadOnlySpan<char> AsSpan(this OpenStringBuilder? text, int start, int length)`
`public static ReadOnlySpan<char> AsSpan(this OpenStringBuilder? text, System.Index startIndex)`
`public static ReadOnlyMemory<char> AsMemory(this OpenStringBuilder? text, System.Index startIndex)`
`public static ReadOnlyMemory<char> AsMemory(this OpenStringBuilder? text, Range range)`
The implementation in `Lucene.Net.MemoryExtensions` is a direct adaption of the original BCL implementation and preserves the optimizations on .NET Core. The tests for them were also derived directly from the BCL.
##########
src/Lucene.Net/Analysis/TokenAttributes/CharTermAttributeImpl.cs:
##########
@@ -516,5 +529,121 @@ public override void CopyTo(IAttribute target) // LUCENENET specific - intention
ISpanAppendable ISpanAppendable.Append(ReadOnlySpan<char> value) => Append(value);
#endregion
+
+ #region IBufferWriter<char> members and implementation support
+
+ // LUCENENET-specific: IBufferWriter<char> support.
+ // See ArrayBufferWriter for an inspiration and reference implementation:
+ // https://github.com/dotnet/runtime/blob/v10.0.6/src/libraries/Common/src/System/Buffers/ArrayBufferWriter.cs
+
+ /// <summary>
+ /// Notifies this instance that <paramref name="count"/> characters were
+ /// written to the buffer returned by the most recent call to
+ /// <see cref="GetSpan(int)"/> or <see cref="GetMemory(int)"/>, advancing
+ /// the term length accordingly.
+ /// <para/>
+ /// LUCENENET specific - implements <see cref="System.Buffers.IBufferWriter{T}"/>.
+ /// </summary>
+ /// <param name="count">The number of characters written to the buffer.</param>
+ /// <exception cref="ArgumentException">
+ /// <paramref name="count"/> is negative.
+ /// </exception>
+ /// <exception cref="InvalidOperationException">
+ /// Advancing by <paramref name="count"/> would move past the end of the
+ /// available buffer space.
+ /// </exception>
+ /// <remarks>
+ /// You must request a new buffer after calling Advance to continue writing more data
+ /// and cannot write to a previously acquired buffer.
+ /// </remarks>
+ public void Advance(int count)
+ {
+ if (count < 0)
+ throw new ArgumentException(null, nameof(count));
Review Comment:
Please throw `ArgumentOutOfRangeException` here.
##########
src/Lucene.Net/Analysis/TokenAttributes/CharTermAttributeImpl.cs:
##########
@@ -516,5 +529,121 @@ public override void CopyTo(IAttribute target) // LUCENENET specific - intention
ISpanAppendable ISpanAppendable.Append(ReadOnlySpan<char> value) => Append(value);
#endregion
+
+ #region IBufferWriter<char> members and implementation support
+
+ // LUCENENET-specific: IBufferWriter<char> support.
+ // See ArrayBufferWriter for an inspiration and reference implementation:
+ // https://github.com/dotnet/runtime/blob/v10.0.6/src/libraries/Common/src/System/Buffers/ArrayBufferWriter.cs
+
+ /// <summary>
+ /// Notifies this instance that <paramref name="count"/> characters were
+ /// written to the buffer returned by the most recent call to
+ /// <see cref="GetSpan(int)"/> or <see cref="GetMemory(int)"/>, advancing
+ /// the term length accordingly.
+ /// <para/>
+ /// LUCENENET specific - implements <see cref="System.Buffers.IBufferWriter{T}"/>.
+ /// </summary>
+ /// <param name="count">The number of characters written to the buffer.</param>
+ /// <exception cref="ArgumentException">
+ /// <paramref name="count"/> is negative.
+ /// </exception>
+ /// <exception cref="InvalidOperationException">
+ /// Advancing by <paramref name="count"/> would move past the end of the
+ /// available buffer space.
+ /// </exception>
+ /// <remarks>
+ /// You must request a new buffer after calling Advance to continue writing more data
+ /// and cannot write to a previously acquired buffer.
+ /// </remarks>
+ public void Advance(int count)
+ {
+ if (count < 0)
+ throw new ArgumentException(null, nameof(count));
+
+ if (termLength > termBuffer.Length - count)
+ throw new InvalidOperationException($"Cannot advance {count} characters beyond the end of the buffer.");
+
+ termLength += count;
+ }
+
+ /// <summary>
+ /// Returns a <see cref="Memory{T}"/> to write to, starting at the current
+ /// term length, that is at least <paramref name="sizeHint"/> characters in
+ /// length. The buffer is grown if necessary; the returned memory is never
+ /// empty.
+ /// <para/>
+ /// LUCENENET specific - implements <see cref="System.Buffers.IBufferWriter{T}"/>.
+ /// </summary>
+ /// <param name="sizeHint">
+ /// The minimum requested length of the returned <see cref="Memory{T}"/>.
+ /// A value of <c>0</c> requests a non-empty buffer of unspecified size.
+ /// </param>
+ /// <returns>
+ /// A <see cref="Memory{T}"/> of at least <paramref name="sizeHint"/>
+ /// characters (and at least one character).
+ /// </returns>
+ /// <exception cref="ArgumentException">
+ /// <paramref name="sizeHint"/> is negative.
+ /// </exception>
+ public Memory<char> GetMemory(int sizeHint = 0)
+ {
+ CheckAndResizeBufferForBufferWriter(sizeHint);
+ Debug.Assert(termBuffer.Length > termLength);
+ return termBuffer.AsMemory(termLength);
+ }
+
+ /// <summary>
+ /// Returns a <see cref="Span{T}"/> to write to, starting at the current
+ /// term length, that is at least <paramref name="sizeHint"/> characters in
+ /// length. The buffer is grown if necessary; the returned span is never
+ /// empty.
+ /// <para/>
+ /// LUCENENET specific - implements <see cref="System.Buffers.IBufferWriter{T}"/>.
+ /// </summary>
+ /// <param name="sizeHint">
+ /// The minimum requested length of the returned <see cref="Span{T}"/>.
+ /// A value of <c>0</c> requests a non-empty buffer of unspecified size.
+ /// </param>
+ /// <returns>
+ /// A <see cref="Span{T}"/> of at least <paramref name="sizeHint"/>
+ /// characters (and at least one character).
+ /// </returns>
+ /// <exception cref="ArgumentException">
+ /// <paramref name="sizeHint"/> is negative.
+ /// </exception>
+ public Span<char> GetSpan(int sizeHint = 0)
+ {
+ CheckAndResizeBufferForBufferWriter(sizeHint);
+ Debug.Assert(termBuffer.Length > termLength);
+ return termBuffer.AsSpan(termLength);
+ }
+
+ private void CheckAndResizeBufferForBufferWriter(int sizeHint)
+ {
+ if (sizeHint < 0)
+ {
+ throw new ArgumentException("sizeHint must be non-negative", nameof(sizeHint));
Review Comment:
Please throw `ArgumentOutOfRangeException` here.
##########
src/Lucene.Net/Analysis/TokenAttributes/CharTermAttributeImpl.cs:
##########
@@ -516,5 +529,121 @@ public override void CopyTo(IAttribute target) // LUCENENET specific - intention
ISpanAppendable ISpanAppendable.Append(ReadOnlySpan<char> value) => Append(value);
#endregion
+
+ #region IBufferWriter<char> members and implementation support
+
+ // LUCENENET-specific: IBufferWriter<char> support.
+ // See ArrayBufferWriter for an inspiration and reference implementation:
+ // https://github.com/dotnet/runtime/blob/v10.0.6/src/libraries/Common/src/System/Buffers/ArrayBufferWriter.cs
+
+ /// <summary>
+ /// Notifies this instance that <paramref name="count"/> characters were
+ /// written to the buffer returned by the most recent call to
+ /// <see cref="GetSpan(int)"/> or <see cref="GetMemory(int)"/>, advancing
+ /// the term length accordingly.
+ /// <para/>
+ /// LUCENENET specific - implements <see cref="System.Buffers.IBufferWriter{T}"/>.
+ /// </summary>
+ /// <param name="count">The number of characters written to the buffer.</param>
+ /// <exception cref="ArgumentException">
+ /// <paramref name="count"/> is negative.
+ /// </exception>
+ /// <exception cref="InvalidOperationException">
+ /// Advancing by <paramref name="count"/> would move past the end of the
+ /// available buffer space.
+ /// </exception>
+ /// <remarks>
+ /// You must request a new buffer after calling Advance to continue writing more data
+ /// and cannot write to a previously acquired buffer.
+ /// </remarks>
+ public void Advance(int count)
+ {
+ if (count < 0)
+ throw new ArgumentException(null, nameof(count));
+
+ if (termLength > termBuffer.Length - count)
+ throw new InvalidOperationException($"Cannot advance {count} characters beyond the end of the buffer.");
+
+ termLength += count;
+ }
+
+ /// <summary>
+ /// Returns a <see cref="Memory{T}"/> to write to, starting at the current
+ /// term length, that is at least <paramref name="sizeHint"/> characters in
+ /// length. The buffer is grown if necessary; the returned memory is never
+ /// empty.
+ /// <para/>
+ /// LUCENENET specific - implements <see cref="System.Buffers.IBufferWriter{T}"/>.
+ /// </summary>
+ /// <param name="sizeHint">
+ /// The minimum requested length of the returned <see cref="Memory{T}"/>.
+ /// A value of <c>0</c> requests a non-empty buffer of unspecified size.
+ /// </param>
+ /// <returns>
+ /// A <see cref="Memory{T}"/> of at least <paramref name="sizeHint"/>
+ /// characters (and at least one character).
+ /// </returns>
+ /// <exception cref="ArgumentException">
+ /// <paramref name="sizeHint"/> is negative.
+ /// </exception>
+ public Memory<char> GetMemory(int sizeHint = 0)
+ {
+ CheckAndResizeBufferForBufferWriter(sizeHint);
+ Debug.Assert(termBuffer.Length > termLength);
+ return termBuffer.AsMemory(termLength);
+ }
+
+ /// <summary>
+ /// Returns a <see cref="Span{T}"/> to write to, starting at the current
+ /// term length, that is at least <paramref name="sizeHint"/> characters in
+ /// length. The buffer is grown if necessary; the returned span is never
+ /// empty.
+ /// <para/>
+ /// LUCENENET specific - implements <see cref="System.Buffers.IBufferWriter{T}"/>.
+ /// </summary>
+ /// <param name="sizeHint">
+ /// The minimum requested length of the returned <see cref="Span{T}"/>.
+ /// A value of <c>0</c> requests a non-empty buffer of unspecified size.
+ /// </param>
+ /// <returns>
+ /// A <see cref="Span{T}"/> of at least <paramref name="sizeHint"/>
+ /// characters (and at least one character).
+ /// </returns>
+ /// <exception cref="ArgumentException">
+ /// <paramref name="sizeHint"/> is negative.
+ /// </exception>
+ public Span<char> GetSpan(int sizeHint = 0)
+ {
+ CheckAndResizeBufferForBufferWriter(sizeHint);
+ Debug.Assert(termBuffer.Length > termLength);
+ return termBuffer.AsSpan(termLength);
+ }
+
+ private void CheckAndResizeBufferForBufferWriter(int sizeHint)
+ {
+ if (sizeHint < 0)
+ {
+ throw new ArgumentException("sizeHint must be non-negative", nameof(sizeHint));
+ }
+
+ if (sizeHint == 0)
+ {
+ sizeHint = 1;
Review Comment:
In this case we have a `MIN_BUFFER_SIZE` that we can use here as a logical default size.
##########
src/Lucene.Net.Tests.Analysis.Common/Analysis/Util/TestOpenStringBuilderBufferWriter.cs:
##########
@@ -0,0 +1,488 @@
+// Adapted from MIT-licensed ArrayBufferWriter tests here:
Review Comment:
Please move this file into a top-level `Support` folder, so it is in `Support/Analysis/Util/`. But leave the namespace `Lucene.Net.Analysis.Util`.
##########
src/Lucene.Net/Analysis/TokenAttributes/CharTermAttributeImpl.cs:
##########
@@ -516,5 +529,121 @@ public override void CopyTo(IAttribute target) // LUCENENET specific - intention
ISpanAppendable ISpanAppendable.Append(ReadOnlySpan<char> value) => Append(value);
#endregion
+
+ #region IBufferWriter<char> members and implementation support
+
+ // LUCENENET-specific: IBufferWriter<char> support.
+ // See ArrayBufferWriter for an inspiration and reference implementation:
+ // https://github.com/dotnet/runtime/blob/v10.0.6/src/libraries/Common/src/System/Buffers/ArrayBufferWriter.cs
+
+ /// <summary>
+ /// Notifies this instance that <paramref name="count"/> characters were
+ /// written to the buffer returned by the most recent call to
+ /// <see cref="GetSpan(int)"/> or <see cref="GetMemory(int)"/>, advancing
+ /// the term length accordingly.
+ /// <para/>
+ /// LUCENENET specific - implements <see cref="System.Buffers.IBufferWriter{T}"/>.
+ /// </summary>
+ /// <param name="count">The number of characters written to the buffer.</param>
+ /// <exception cref="ArgumentException">
+ /// <paramref name="count"/> is negative.
+ /// </exception>
+ /// <exception cref="InvalidOperationException">
+ /// Advancing by <paramref name="count"/> would move past the end of the
+ /// available buffer space.
+ /// </exception>
+ /// <remarks>
+ /// You must request a new buffer after calling Advance to continue writing more data
+ /// and cannot write to a previously acquired buffer.
+ /// </remarks>
+ public void Advance(int count)
+ {
+ if (count < 0)
+ throw new ArgumentException(null, nameof(count));
+
+ if (termLength > termBuffer.Length - count)
Review Comment:
As in the other implementation, I think the intent is clearer by putting count on the left side.
```c#
if (count > termBuffer.Length - termLength)
```
##########
src/Lucene.Net.Analysis.Common/Analysis/Util/OpenStringBuilder.cs:
##########
@@ -400,5 +416,72 @@ IAppendable IAppendable.Append(ICharSequence value, int startIndex, int count)
ISpanAppendable ISpanAppendable.Append(ReadOnlySpan<char> value) => Append(value);
#endregion IAppendable Members
+
+ #region IBufferWriter<char> members and support
+
+ // LUCENENET-specific: IBufferWriter<char> support.
+ // See ArrayBufferWriter for an inspiration and reference implementation:
+ // https://github.com/dotnet/runtime/blob/v10.0.6/src/libraries/Common/src/System/Buffers/ArrayBufferWriter.cs
+
+ public void Advance(int count)
+ {
+ if (count < 0)
+ throw new ArgumentException(null, nameof(count));
Review Comment:
Let's throw `ArgumentOutOfRangeException` here.
##########
src/Lucene.Net.Analysis.Common/Analysis/Util/OpenStringBuilder.cs:
##########
@@ -400,5 +416,72 @@ IAppendable IAppendable.Append(ICharSequence value, int startIndex, int count)
ISpanAppendable ISpanAppendable.Append(ReadOnlySpan<char> value) => Append(value);
#endregion IAppendable Members
+
+ #region IBufferWriter<char> members and support
+
+ // LUCENENET-specific: IBufferWriter<char> support.
+ // See ArrayBufferWriter for an inspiration and reference implementation:
+ // https://github.com/dotnet/runtime/blob/v10.0.6/src/libraries/Common/src/System/Buffers/ArrayBufferWriter.cs
+
+ public void Advance(int count)
+ {
+ if (count < 0)
+ throw new ArgumentException(null, nameof(count));
+
+ if (m_len > m_buf.Length - count)
+ throw new InvalidOperationException($"Cannot advance {count} characters beyond the end of the buffer.");
+
+ m_len += count;
+ }
+
+ public Memory<char> GetMemory(int sizeHint = 0)
+ {
+ CheckAndResizeBufferForBufferWriter(sizeHint);
+ Debug.Assert(m_buf.Length > m_len);
+ return m_buf.AsMemory(m_len);
+ }
+
+ public Span<char> GetSpan(int sizeHint = 0)
+ {
+ CheckAndResizeBufferForBufferWriter(sizeHint);
+ Debug.Assert(m_buf.Length > m_len);
+ return m_buf.AsSpan(m_len);
+ }
+
+ private void CheckAndResizeBufferForBufferWriter(int sizeHint)
+ {
+ if (sizeHint < 0)
+ {
+ throw new ArgumentException("sizeHint must be non-negative", nameof(sizeHint));
Review Comment:
Please throw `ArgumentOutOfRangeException` here.
##########
src/Lucene.Net.Tests/Analysis/TokenAttributes/TestCharTermAttributeImplBufferWriter.cs:
##########
@@ -0,0 +1,445 @@
+// Adapted from MIT-licensed ArrayBufferWriter tests here:
Review Comment:
Please move this file into a top-level `Support` folder, so it is in `Support/Analysis/TokenAttributes/`. But leave the namespace `Lucene.Net.Analysis.TokenAttributes`.
##########
src/Lucene.Net.Analysis.Common/Analysis/Util/OpenStringBuilder.cs:
##########
@@ -400,5 +416,72 @@ IAppendable IAppendable.Append(ICharSequence value, int startIndex, int count)
ISpanAppendable ISpanAppendable.Append(ReadOnlySpan<char> value) => Append(value);
#endregion IAppendable Members
+
+ #region IBufferWriter<char> members and support
+
+ // LUCENENET-specific: IBufferWriter<char> support.
+ // See ArrayBufferWriter for an inspiration and reference implementation:
+ // https://github.com/dotnet/runtime/blob/v10.0.6/src/libraries/Common/src/System/Buffers/ArrayBufferWriter.cs
+
+ public void Advance(int count)
+ {
+ if (count < 0)
+ throw new ArgumentException(null, nameof(count));
+
+ if (m_len > m_buf.Length - count)
Review Comment:
I think the intent would be clearer if the `count` and `m_len` variables were reversed here.
```c#
if (count > m_buf.Lenth - m_len)
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@lucenenet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org