Skip to content
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

Jump to bottom

Improve performance of polymorphism #42538

Merged
merged 9 commits into from
Oct 19, 2020
Merged

Improve performance of polymorphism #42538

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
d359119
Improve performance of polymorphism
steveharter Sep 18, 2020
af01d59
Add additional caching; avoid more allocs; feedback
steveharter Sep 25, 2020
d034ae8
Avoid some pool usage in non-serializer path; address feedback
steveharter Sep 29, 2020
cafef3b
Encapsulate logic better for StackRowStack
steveharter Sep 30, 2020
2a545d9
Merge branch 'master' into JsonElementPerf
steveharter Sep 30, 2020
c36687f
Minor refactoring for perf
steveharter Oct 9, 2020
8b370ca
Make WSF.PolymorphicJsonPropertyInfo private and add comments
steveharter Oct 14, 2020
d4bdb19
Misc feedback
steveharter Oct 16, 2020
48db4b9
Add back Enlarge(); update Asserts
steveharter Oct 19, 2020
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
6 changes: 6 additions & 0 deletions src/libraries/System.Text.Json/src/System/Text/Json/Document/JsonDocument.MetadataDb.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,8 +86,14 @@ private struct MetadataDb : IDisposable

internal int Length { get; private set; }
private byte[] _data;

private bool _convertToAlloc; // Convert the rented data to an alloc when complete.
private bool _isLocked; // Is the array the correct fixed size.
steveharter marked this conversation as resolved.
Show resolved Hide resolved
// _isLocked _convertToAlloc truth table:
// false false Standard flow. Size is not known and renting used throughout lifetime.
// true false Used by JsonElement.ParseValue() for primitives and JsonDocument.Clone(). Size is known and no renting.
// false true Used by JsonElement.ParseValue() for arrays and objects. Renting used until size is known.
// true true not valid

private MetadataDb(byte[] initialDb, bool isLocked, bool convertToAlloc)
{
Expand Down
24 changes: 19 additions & 5 deletions src/libraries/System.Text.Json/src/System/Text/Json/Document/JsonDocument.Parse.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -616,16 +616,21 @@ private static JsonDocument Parse(
{
ReadOnlySpan<byte> utf8JsonSpan = utf8Json.Span;
var database = MetadataDb.CreateRented(utf8Json.Length, convertToAlloc: false);
var stack = new StackRowStack(JsonDocumentOptions.DefaultMaxDepth * StackRow.Size);

try
{
Parse(utf8JsonSpan, readerOptions, ref database);
Parse(utf8JsonSpan, readerOptions, ref database, ref stack);
}
catch
{
database.Dispose();
throw;
}
finally
{
stack.Dispose();
}

return new JsonDocument(utf8Json, database, extraRentedBytes);
}
Expand All @@ -644,18 +649,27 @@ private static JsonDocument ParseUnrented(
ReadOnlySpan<byte> utf8JsonSpan = utf8Json.Span;
MetadataDb database;

if (tokenType == JsonTokenType.String ||
tokenType == JsonTokenType.Number)
if (tokenType == JsonTokenType.String || tokenType == JsonTokenType.Number)
{
// For primitive types, we can avoid renting.
// For primitive types, we can avoid renting MetadataDb and creating StackRowStack.
database = MetadataDb.CreateLocked(utf8Json.Length);
StackRowStack stack = default;
Parse(utf8JsonSpan, readerOptions, ref database, ref stack);
}
else
{
database = MetadataDb.CreateRented(utf8Json.Length, convertToAlloc: true);
var stack = new StackRowStack(JsonDocumentOptions.DefaultMaxDepth * StackRow.Size);
try
{
Parse(utf8JsonSpan, readerOptions, ref database, ref stack);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How's test coverage for all these changes? Are we still at 100% for JsonElement/JsonDocument?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The only new LOC that isn't hit is this branch: https://github.com/dotnet/runtime/pull/42538/files#diff-8bee6ed7017869dde2a37df1271e0338R612.

                if (valueSpan.IsEmpty)
                {
                    owned = valueSequence.ToArray();
                }

This branch is only hit if we expose this functionality publicly (not called from the serializer) since "valueSpan" will never be empty when using a reader instance with the serializer.

So we could remove this branch and add an Assert, but I suggest leaving until we decide on #42755 which at that point we will add tests to cover that.

}
finally
{
stack.Dispose();
}
}

Parse(utf8JsonSpan, readerOptions, ref database);
return new JsonDocument(utf8Json, database, extraRentedBytes: null);
}

Expand Down
18 changes: 5 additions & 13 deletions src/libraries/System.Text.Json/src/System/Text/Json/Document/JsonDocument.StackRowStack.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,21 +11,18 @@ public sealed partial class JsonDocument
{
private struct StackRowStack : IDisposable
{
private byte[]? _rentedBuffer;
private byte[] _rentedBuffer;
private int _topOfStack;

public void Initialize(int initialSize)
public StackRowStack(int initialSize)
{
_rentedBuffer = ArrayPool<byte>.Shared.Rent(initialSize);
_topOfStack = _rentedBuffer.Length;
Debug.Assert(IsInitialized);
}

private bool IsInitialized { get { return _rentedBuffer != null; } }

public void Dispose()
{
byte[]? toReturn = _rentedBuffer;
byte[] toReturn = _rentedBuffer;
_rentedBuffer = null!;
_topOfStack = 0;

Expand All @@ -40,11 +37,6 @@ public void Dispose()

internal void Push(StackRow row)
{
if (!IsInitialized)
{
Initialize(JsonDocumentOptions.DefaultMaxDepth * StackRow.Size);
}

if (_topOfStack < StackRow.Size)
{
Enlarge();
Expand All @@ -56,7 +48,7 @@ internal void Push(StackRow row)

internal StackRow Pop()
{
Debug.Assert(IsInitialized);
Debug.Assert(_rentedBuffer != null);
Debug.Assert(_topOfStack <= _rentedBuffer!.Length - StackRow.Size);

StackRow row = MemoryMarshal.Read<StackRow>(_rentedBuffer.AsSpan(_topOfStack));
Expand All @@ -66,7 +58,7 @@ internal StackRow Pop()

private void Enlarge()
{
byte[] toReturn = _rentedBuffer!;
byte[] toReturn = _rentedBuffer;
_rentedBuffer = ArrayPool<byte>.Shared.Rent(toReturn.Length * 2);

Buffer.BlockCopy(
Expand Down
214 changes: 105 additions & 109 deletions src/libraries/System.Text.Json/src/System/Text/Json/Document/JsonDocument.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -919,7 +919,8 @@ private void WriteString(in DbRow row, Utf8JsonWriter writer)
private static void Parse(
ReadOnlySpan<byte> utf8JsonSpan,
JsonReaderOptions readerOptions,
ref MetadataDb database)
ref MetadataDb database,
ref StackRowStack stack)
steveharter marked this conversation as resolved.
Show resolved Hide resolved
{
bool inArray = false;
int arrayItemsCount = 0;
Expand All @@ -931,98 +932,120 @@ private static void Parse(
isFinalBlock: true,
new JsonReaderState(options: readerOptions));

StackRowStack stack = default;
using (stack)
while (reader.Read())
{
while (reader.Read())
{
JsonTokenType tokenType = reader.TokenType;
JsonTokenType tokenType = reader.TokenType;

// Since the input payload is contained within a Span,
// token start index can never be larger than int.MaxValue (i.e. utf8JsonSpan.Length).
Debug.Assert(reader.TokenStartIndex <= int.MaxValue);
int tokenStart = (int)reader.TokenStartIndex;
// Since the input payload is contained within a Span,
// token start index can never be larger than int.MaxValue (i.e. utf8JsonSpan.Length).
Debug.Assert(reader.TokenStartIndex <= int.MaxValue);
int tokenStart = (int)reader.TokenStartIndex;

if (tokenType == JsonTokenType.StartObject)
if (tokenType == JsonTokenType.StartObject)
{
if (inArray)
{
if (inArray)
{
arrayItemsCount++;
}

numberOfRowsForValues++;
database.Append(tokenType, tokenStart, DbRow.UnknownSize);
var row = new StackRow(numberOfRowsForMembers + 1);
stack.Push(row);
numberOfRowsForMembers = 0;
arrayItemsCount++;
}
else if (tokenType == JsonTokenType.EndObject)
{
int rowIndex = database.FindIndexOfFirstUnsetSizeOrLength(JsonTokenType.StartObject);

numberOfRowsForValues++;
numberOfRowsForMembers++;
database.SetLength(rowIndex, numberOfRowsForMembers);
numberOfRowsForValues++;
database.Append(tokenType, tokenStart, DbRow.UnknownSize);
var row = new StackRow(numberOfRowsForMembers + 1);
stack.Push(row);
numberOfRowsForMembers = 0;
}
else if (tokenType == JsonTokenType.EndObject)
{
int rowIndex = database.FindIndexOfFirstUnsetSizeOrLength(JsonTokenType.StartObject);

int newRowIndex = database.Length;
database.Append(tokenType, tokenStart, reader.ValueSpan.Length);
database.SetNumberOfRows(rowIndex, numberOfRowsForMembers);
database.SetNumberOfRows(newRowIndex, numberOfRowsForMembers);
numberOfRowsForValues++;
numberOfRowsForMembers++;
database.SetLength(rowIndex, numberOfRowsForMembers);

StackRow row = stack.Pop();
numberOfRowsForMembers += row.SizeOrLength;
}
else if (tokenType == JsonTokenType.StartArray)
{
if (inArray)
{
arrayItemsCount++;
}
int newRowIndex = database.Length;
database.Append(tokenType, tokenStart, reader.ValueSpan.Length);
database.SetNumberOfRows(rowIndex, numberOfRowsForMembers);
database.SetNumberOfRows(newRowIndex, numberOfRowsForMembers);

numberOfRowsForMembers++;
database.Append(tokenType, tokenStart, DbRow.UnknownSize);
var row = new StackRow(arrayItemsCount, numberOfRowsForValues + 1);
stack.Push(row);
arrayItemsCount = 0;
numberOfRowsForValues = 0;
StackRow row = stack.Pop();
numberOfRowsForMembers += row.SizeOrLength;
}
else if (tokenType == JsonTokenType.StartArray)
{
if (inArray)
{
arrayItemsCount++;
}
else if (tokenType == JsonTokenType.EndArray)

numberOfRowsForMembers++;
database.Append(tokenType, tokenStart, DbRow.UnknownSize);
var row = new StackRow(arrayItemsCount, numberOfRowsForValues + 1);
stack.Push(row);
arrayItemsCount = 0;
numberOfRowsForValues = 0;
}
else if (tokenType == JsonTokenType.EndArray)
{
int rowIndex = database.FindIndexOfFirstUnsetSizeOrLength(JsonTokenType.StartArray);

numberOfRowsForValues++;
numberOfRowsForMembers++;
database.SetLength(rowIndex, arrayItemsCount);
database.SetNumberOfRows(rowIndex, numberOfRowsForValues);

// If the array item count is (e.g.) 12 and the number of rows is (e.g.) 13
// then the extra row is just this EndArray item, so the array was made up
// of simple values.
//
// If the off-by-one relationship does not hold, then one of the values was
// more than one row, making it a complex object.
//
// This check is similar to tracking the start array and painting it when
// StartObject or StartArray is encountered, but avoids the mixed state
// where "UnknownSize" implies "has complex children".
if (arrayItemsCount + 1 != numberOfRowsForValues)
{
int rowIndex = database.FindIndexOfFirstUnsetSizeOrLength(JsonTokenType.StartArray);

numberOfRowsForValues++;
numberOfRowsForMembers++;
database.SetLength(rowIndex, arrayItemsCount);
database.SetNumberOfRows(rowIndex, numberOfRowsForValues);

// If the array item count is (e.g.) 12 and the number of rows is (e.g.) 13
// then the extra row is just this EndArray item, so the array was made up
// of simple values.
//
// If the off-by-one relationship does not hold, then one of the values was
// more than one row, making it a complex object.
//
// This check is similar to tracking the start array and painting it when
// StartObject or StartArray is encountered, but avoids the mixed state
// where "UnknownSize" implies "has complex children".
if (arrayItemsCount + 1 != numberOfRowsForValues)
{
database.SetHasComplexChildren(rowIndex);
}
database.SetHasComplexChildren(rowIndex);
}

int newRowIndex = database.Length;
database.Append(tokenType, tokenStart, reader.ValueSpan.Length);
database.SetNumberOfRows(newRowIndex, numberOfRowsForValues);
int newRowIndex = database.Length;
database.Append(tokenType, tokenStart, reader.ValueSpan.Length);
database.SetNumberOfRows(newRowIndex, numberOfRowsForValues);

StackRow row = stack.Pop();
arrayItemsCount = row.SizeOrLength;
numberOfRowsForValues += row.NumberOfRows;
}
else if (tokenType == JsonTokenType.PropertyName)
{
numberOfRowsForValues++;
numberOfRowsForMembers++;

// Adding 1 to skip the start quote will never overflow
Debug.Assert(tokenStart < int.MaxValue);

StackRow row = stack.Pop();
arrayItemsCount = row.SizeOrLength;
numberOfRowsForValues += row.NumberOfRows;
database.Append(tokenType, tokenStart + 1, reader.ValueSpan.Length);

if (reader._stringHasEscaping)
{
database.SetHasComplexChildren(database.Length - DbRow.Size);
}
else if (tokenType == JsonTokenType.PropertyName)

Debug.Assert(!inArray);
}
else
{
Debug.Assert(tokenType >= JsonTokenType.String && tokenType <= JsonTokenType.Null);
numberOfRowsForValues++;
numberOfRowsForMembers++;

if (inArray)
{
numberOfRowsForValues++;
numberOfRowsForMembers++;
arrayItemsCount++;
}

if (tokenType == JsonTokenType.String)
{
// Adding 1 to skip the start quote will never overflow
Debug.Assert(tokenStart < int.MaxValue);

Expand All @@ -1032,45 +1055,18 @@ private static void Parse(
{
database.SetHasComplexChildren(database.Length - DbRow.Size);
}

Debug.Assert(!inArray);
}
else
{
Debug.Assert(tokenType >= JsonTokenType.String && tokenType <= JsonTokenType.Null);
numberOfRowsForValues++;
numberOfRowsForMembers++;

if (inArray)
{
arrayItemsCount++;
}

if (tokenType == JsonTokenType.String)
{
// Adding 1 to skip the start quote will never overflow
Debug.Assert(tokenStart < int.MaxValue);

database.Append(tokenType, tokenStart + 1, reader.ValueSpan.Length);

if (reader._stringHasEscaping)
{
database.SetHasComplexChildren(database.Length - DbRow.Size);
}
}
else
{
database.Append(tokenType, tokenStart, reader.ValueSpan.Length);
}
database.Append(tokenType, tokenStart, reader.ValueSpan.Length);
}

inArray = reader.IsInArray;
}

Debug.Assert(reader.BytesConsumed == utf8JsonSpan.Length);

database.CompleteAllocations();
inArray = reader.IsInArray;
}

Debug.Assert(reader.BytesConsumed == utf8JsonSpan.Length);
database.CompleteAllocations();
}

private void CheckNotDisposed()
Expand Down