Skip to content

SequenceReader performance; elide bounds checks and book-keeping #82765

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
mgravell wants to merge 23 commits into from
Closed

SequenceReader performance; elide bounds checks and book-keeping #82765

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
580a164
SequenceReader; elide bounds checks by using ref-oriented approach
mgravell Feb 28, 2023
dfe7241
net6-: don't forget to increment _consumedAtStartOfCurrentSpan
mgravell Feb 28, 2023
481b49c
- don't use the Unsafe or MemoryMarshal evilness
mgravell Mar 1, 2023
a5bf93d
in search, avoid copying out the T value
mgravell Mar 1, 2023
836bc9c
drop unused method
mgravell Mar 1, 2023
4e8d89c
reinstate next position; needed to avoid double-fetch in GetNextSpan
mgravell Mar 1, 2023
f359dd0
use better API to get first span in ResetReader
mgravell Mar 1, 2023
454315e
simplify Position getter
mgravell Mar 1, 2023
d001302
move reset logic to constructor; use constructor in ResetReader
mgravell Mar 1, 2023
4ba4be6
always track consumed span when moving to next, even for single segment
mgravell Mar 1, 2023
ec519f7
for single-segment cases, length is trivial; record length at EOF; up…
mgravell Mar 1, 2023
f6d17c4
use named parameter to clarify meaning
mgravell Mar 1, 2023
7a17388
explicit types (IDE0008)
mgravell Mar 1, 2023
4781155
- fix increment bug at end of single-segment buffers
mgravell Mar 2, 2023
e49b863
simplify advance further
mgravell Mar 2, 2023
11a7b46
remove debug-check from AssertValidPosition() to make the compiler ha…
mgravell Mar 2, 2023
570d221
TryPeek/TryRead: avoid bounds check by hoisting into locals
mgravell Mar 2, 2023
c1c6d59
nit: use local for the addition too (cuts a `mov` from the JIT output)
mgravell Mar 2, 2023
594f879
test fixes
mgravell Mar 2, 2023
892a9e7
Advance: don't short-circuit (we expect both sides to pass)
mgravell Mar 3, 2023
eeb4bff
work around JIT regressions
mgravell Mar 3, 2023
0412ab1
nit: TryGetNextBuffer - defer updating result; empty segments are v.rare
mgravell Mar 7, 2023
e825679
Fix two rebase errors
jozkee Aug 17, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
reinstate next position; needed to avoid double-fetch in GetNextSpan
  • Loading branch information
@mgravell @jozkee
mgravell authored and jozkee committed Aug 11, 2023
commit 4e8d89c8ad0c15539579c03e865e05930720b488
59 changes: 27 additions & 32 deletions src/libraries/System.Memory/src/System/Buffers/SequenceReader.Search.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -755,6 +755,9 @@ public void AdvanceToEnd()
var position = _sequence.End;
_currentPositionObject = position.GetObject();
_currentPositionInteger = position.GetInteger();
// and mimic default(Position) for next
_nextPositionObject = default;
_nextPositionInteger = default;
}
}

Expand Down Expand Up @@ -810,46 +813,38 @@ private bool IsNextSlow(scoped ReadOnlySpan<T> next, bool advancePast)
Debug.Assert(currentSpan.Length < next.Length);

int fullLength = next.Length;
SequencePosition position = new(_currentPositionObject, _currentPositionInteger);
if (_sequence.TryGetBuffer(position, out var nextSegment, out var nextPosition))
{
// no-op removed by compiler; we don't need the first segment, but we
// also don't want the compiler creating a hidden local for a discard
// (we use this same local *later*)
_ = nextSegment; // suppress IDE0059
SequencePosition position = new(_nextPositionObject, _nextPositionInteger);

position = nextPosition;
while (next.StartsWith(currentSpan))
while (next.StartsWith(currentSpan))
{
if (next.Length == currentSpan.Length)
{
if (next.Length == currentSpan.Length)
// Fully matched
if (advancePast)
{
// Fully matched
if (advancePast)
{
Advance(fullLength);
}
return true;
Advance(fullLength);
}
return true;
}

// Need to check the next segment
while (true)
// Need to check the next segment
while (true)
{
if (!_sequence.TryGetBuffer(position, out var nextSegment, out var nextPosition))
{
if (!_sequence.TryGetBuffer(position, out nextSegment, out nextPosition))
{
// Nothing left
return false;
}
position = nextPosition;
if (nextSegment.Length > 0)
// Nothing left
return false;
}
position = nextPosition;
if (nextSegment.Length > 0)
{
next = next.Slice(currentSpan.Length);
currentSpan = nextSegment.Span;
if (currentSpan.Length > next.Length)
{
next = next.Slice(currentSpan.Length);
currentSpan = nextSegment.Span;
if (currentSpan.Length > next.Length)
{
currentSpan = currentSpan.Slice(0, next.Length);
}
break;
currentSpan = currentSpan.Slice(0, next.Length);
}
break;
}
}
}
Expand Down
77 changes: 34 additions & 43 deletions src/libraries/System.Memory/src/System/Buffers/SequenceReader.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,11 @@ namespace System.Buffers
{
public ref partial struct SequenceReader<T> where T : unmanaged, IEquatable<T>
{
// keep all fields explicit to track (and pack) space; currently 72 bytes on x64
// keep all fields explicit to track (and pack) space

// deconstruct position for packing purposes
private object? _currentPositionObject;
private int _currentPositionInteger, _currentSpanIndex;
private object? _currentPositionObject, _nextPositionObject;
private int _currentPositionInteger, _nextPositionInteger, _currentSpanIndex;

private readonly long _length;
private long _consumedAtStartOfCurrentSpan;
Expand Down Expand Up @@ -153,28 +153,23 @@ public readonly bool TryPeek(long offset, out T value)
else
{
long remainingOffset = offset - (_currentSpan.Length - _currentSpanIndex);
SequencePosition position = new(_currentPositionObject, _currentPositionInteger);
SequencePosition position = new(_nextPositionObject, _nextPositionInteger);

// first we need to skip past the current span (we already discounted that data)
if (_sequence.TryGetBuffer(position, out var currentMemory, out var next))
ReadOnlyMemory<T> currentMemory;
while (_sequence.TryGetBuffer(position, out currentMemory, out var next))
{
// now seek the additional buffers until we land in the segment we want
position = next;
while (_sequence.TryGetBuffer(position, out currentMemory, out next))
// Skip empty segment
if (currentMemory.Length > 0)
{
position = next;
// Skip empty segment
if (currentMemory.Length > 0)
if (remainingOffset >= currentMemory.Length)
{
if (remainingOffset >= currentMemory.Length)
{
// Subtract current non consumed data
remainingOffset -= currentMemory.Length;
}
else
{
break;
}
// Subtract current non consumed data
remainingOffset -= currentMemory.Length;
}
else
{
break;
}
}
}
Expand Down Expand Up @@ -252,9 +247,12 @@ private void ResetReader()
_currentPositionObject = position.GetObject();
_currentPositionInteger = position.GetInteger();
_consumedAtStartOfCurrentSpan = 0;
_currentSpan = _sequence.FirstSpan;
_sequence.TryGetBuffer(position, out var memory, out var next);
_nextPositionObject = next.GetObject();
_nextPositionInteger = next.GetInteger();
_currentSpan = memory.Span;
_currentSpanIndex = 0;
if (_currentSpanIndex == _currentSpan.Length && !_sequence.IsSingleSegment)
if (_currentSpan.IsEmpty && !_sequence.IsSingleSegment)
{
GetNextSpan();
}
Expand All @@ -269,15 +267,18 @@ private bool GetNextSpan()
{
_consumedAtStartOfCurrentSpan += _currentSpan.Length; // account for previous

SequencePosition position = new(_currentPositionObject, _currentPositionInteger);
while (_sequence.TryGetBuffer(position, out ReadOnlyMemory<T> memory, out var next))
SequencePosition position = new(_nextPositionObject, _nextPositionInteger);

while (_sequence.TryGetBuffer(position, out var memory, out var next))
{
if (memory.Length > 0)
{
_currentSpan = memory.Span;
_currentSpanIndex = 0;
_currentPositionObject = position.GetObject();
_currentPositionInteger = position.GetInteger();
_nextPositionObject = next.GetObject();
_nextPositionInteger = next.GetInteger();
return true;
}
position = next;
Expand Down Expand Up @@ -396,29 +397,19 @@ internal readonly bool TryCopyMultisegment(Span<T> destination)
firstSpan.CopyTo(destination);
int copied = firstSpan.Length;

SequencePosition position = new(_currentPositionObject, _currentPositionInteger);
// first we need to skip past the current span (we already discounted that data)
if (_sequence.TryGetBuffer(position, out var nextSegment, out var next))
SequencePosition position = new(_nextPositionObject, _nextPositionInteger);
while (_sequence.TryGetBuffer(position, out var nextSegment, out var next))
{
// no-op removed by compiler; we don't need the first segment, but we
// also don't want the compiler creating a hidden local for a discard
// (we use this same local *later*)
_ = nextSegment; // suppress IDE0059

position = next;
while (_sequence.TryGetBuffer(position, out nextSegment, out next))
if (nextSegment.Length > 0)
{
position = next;
if (nextSegment.Length > 0)
ReadOnlySpan<T> nextSpan = nextSegment.Span;
int toCopy = Math.Min(nextSpan.Length, destination.Length - copied);
nextSpan.Slice(0, toCopy).CopyTo(destination.Slice(copied));
copied += toCopy;
if (copied >= destination.Length)
{
ReadOnlySpan<T> nextSpan = nextSegment.Span;
int toCopy = Math.Min(nextSpan.Length, destination.Length - copied);
nextSpan.Slice(0, toCopy).CopyTo(destination.Slice(copied));
copied += toCopy;
if (copied >= destination.Length)
{
break;
}
break;
}
}
}
Expand Down