forked from dotnet/corefx
-
Notifications
You must be signed in to change notification settings - Fork 1
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.
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
JsonDocument Working PR #2
Open
bartonjs
wants to merge
54
commits into
master
Choose a base branch
from
json_document
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from 8 commits
Commits
Show all changes
54 commits
Select commit
Hold shift + click to select a range
4a6719d
Initial port of JsonObject (as JsonDocument/JsonElement)
bartonjs 15854b1
Port JsonObjectTests -> JsonDocumentTests
bartonjs 4958d2e
All ported tests pass
bartonjs 12ed363
Off by one error in StartArray
bartonjs 4c2e256
Minor test cleanup
bartonjs 55e51fc
Make ToString work on Object and Array
bartonjs 9c723cd
Initial performance tests for JsonDocument
bartonjs 67765e0
Add some tests to touch every element
bartonjs f3f6a92
HasChildren => HasComplexChildren and use it for a faster indexer
bartonjs b6bcc40
Remove explicit casts in favor of the named methods.
bartonjs 7f0682b
Remove custom pooling from CustomDb since JsonDocument dropped it
bartonjs 8cbb903
TryGetRawData<T> => TryGetRawValue(non-generic)
bartonjs 079fa8b
Remove ported diagnostic message from CustomStack
bartonjs 63270d3
Make JsonElement readonly
bartonjs b24e65f
Address missing test changes
bartonjs 48bc093
Clean up CustomDb
bartonjs 148dd59
Don't need to clear the temporary transcode buffer
bartonjs b340315
Add GetPropertyName and GetPropertyValue
bartonjs 60331b3
Code cleanup in JsonDocument
bartonjs 0b8cc8f
Switch to validating UTF-8 encoder
bartonjs a9bbd87
Make PropertyName.ToString do what was discussed
bartonjs c2e0e6b
Add Parse overloads that work on string and ROS-char
bartonjs 15d0f80
Disallow preserving comments into the DOM
bartonjs ebe1609
Use the default max depth
bartonjs 6b87961
Add more tests
bartonjs 4e4aa16
Make JsonReaderOptions optional
bartonjs 64531a3
Remove TryGetRawValue and TryCopyRawValue
bartonjs 7255c4f
Outline CheckValidInstance
bartonjs 343d706
Outline CheckNotDisposed and CheckExpectedType
bartonjs e44beac
Separate EnumerateArray and EnumerateObject, implement IEnumera-inter…
bartonjs 41f644c
TryGetValue => TryGet[type]
bartonjs c3b0d30
Add uint and float support
bartonjs c14edf5
Add support for ReadOnlySequence<byte>
bartonjs 4dbc498
Move CustomDb initial size calculation into its ctor
bartonjs a106723
Support GetString on JsonTokenType.Null
bartonjs 6a036db
Remove some dead code
bartonjs e209da5
Some missed outlining
bartonjs 58c1e5b
More tests (and fixes)
bartonjs 5cc921b
Make property matching match on the last one
bartonjs 68efe04
Add more tests for the iterators
bartonjs e0800a4
Introduce a new enum for JsonElement.Type
bartonjs b3b2fa8
Add a test that uses multiple object lookups before changing that ind…
bartonjs 8438baa
Improve legibility of fields in DbRow
bartonjs b0ed42c
Replace string indexers with GetProperty methods
bartonjs 8871654
Add JsonElement.[Try]GetDecimal
bartonjs 470449e
Fix optimized indexer for simple arrays
bartonjs 12ef0c5
Ensure coverage for resized database
bartonjs 5492aba
Make JsonProperty a readonly struct
bartonjs ab87420
Add Parse(Stream) and ParseAsync(Stream)
bartonjs bde6e70
Be consistent with the sealed keyword.
bartonjs b5da8ff
Some tweaks to ParseAsync
bartonjs d4131ac
Test it or it's not real.
bartonjs fa850b1
Add JsonElement.GetRawText() and JsonProperty.ToString
bartonjs 069dbc4
SR all the things
bartonjs File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
206 changes: 206 additions & 0 deletions
206
src/System.Text.Json/src/System/Text/Json/JsonDocument.CustomDb.cs
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,206 @@ | ||
// Licensed to the .NET Foundation under one or more agreements. | ||
// The .NET Foundation licenses this file to you under the MIT license. | ||
// See the LICENSE file in the project root for more information. | ||
|
||
using System.Buffers; | ||
using System.Diagnostics; | ||
using System.Runtime.InteropServices; | ||
|
||
namespace System.Text.Json | ||
{ | ||
partial class JsonDocument | ||
{ | ||
private struct CustomDb : IDisposable | ||
bartonjs marked this conversation as resolved.
Show resolved
Hide resolved
|
||
{ | ||
private const int SizeOrLengthOffset = 4; | ||
private const int NumberOfRowsOffset = 8; | ||
|
||
internal int Length; | ||
private byte[] _rentedBuffer; | ||
private ArrayPool<byte> _pool; | ||
bartonjs marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
internal CustomDb(ArrayPool<byte> pool, int initialSize) | ||
{ | ||
_pool = pool; | ||
_rentedBuffer = _pool.Rent(initialSize); | ||
Length = 0; | ||
} | ||
|
||
public void Dispose() | ||
{ | ||
if (_rentedBuffer == null) | ||
{ | ||
return; | ||
} | ||
|
||
_pool.Return(_rentedBuffer); | ||
_rentedBuffer = null; | ||
bartonjs marked this conversation as resolved.
Show resolved
Hide resolved
|
||
_pool = null; | ||
Length = 0; | ||
} | ||
|
||
internal void TrimExcess() | ||
{ | ||
if (Length <= _rentedBuffer.Length / 2) | ||
{ | ||
byte[] newRent = _pool.Rent(Length); | ||
byte[] returnBuf = newRent; | ||
|
||
if (newRent.Length < _rentedBuffer.Length) | ||
{ | ||
Buffer.BlockCopy(_rentedBuffer, 0, newRent, 0, Length); | ||
returnBuf = _rentedBuffer; | ||
_rentedBuffer = newRent; | ||
} | ||
|
||
_pool.Return(returnBuf); | ||
} | ||
} | ||
|
||
internal void Append(JsonTokenType tokenType, int startLocation, int length, bool isPropertyValue) | ||
{ | ||
// StartArray or StartObject should have length -1, otherwise the length should not be -1. | ||
Debug.Assert( | ||
(tokenType == JsonTokenType.StartArray || tokenType == JsonTokenType.StartObject) == | ||
(length == DbRow.UnknownSize)); | ||
|
||
if (Length >= _rentedBuffer.Length - DbRow.Size) | ||
{ | ||
Enlarge(); | ||
} | ||
|
||
DbRow row = new DbRow(tokenType, startLocation, length, isPropertyValue); | ||
MemoryMarshal.Write(_rentedBuffer.AsSpan(Length), ref row); | ||
Length += DbRow.Size; | ||
} | ||
|
||
private void Enlarge() | ||
{ | ||
int size = _rentedBuffer.Length * 2; | ||
byte[] newArray = _pool.Rent(size); | ||
Buffer.BlockCopy(_rentedBuffer, 0, newArray, 0, Length); | ||
_pool.Return(_rentedBuffer); | ||
_rentedBuffer = newArray; | ||
} | ||
|
||
[Conditional("DEBUG")] | ||
private void AssertValidIndex(int index) | ||
{ | ||
Debug.Assert(index >= 0); | ||
Debug.Assert(index <= Length - DbRow.Size, $"index {index} is out of bounds"); | ||
Debug.Assert(index % DbRow.Size == 0, $"index {index} is not at a record start position"); | ||
} | ||
|
||
internal void SetLength(int index, int length) | ||
{ | ||
AssertValidIndex(index); | ||
Debug.Assert(length >= 0); | ||
Span<byte> destination = _rentedBuffer.AsSpan(index + 4); | ||
int cur = MemoryMarshal.Read<int>(destination); | ||
|
||
// Persist the most significant bit | ||
length |= (cur & unchecked((int)0x80000000)); | ||
MemoryMarshal.Write(destination, ref length); | ||
} | ||
|
||
internal void SetNumberOfRows(int index, int numberOfRows) | ||
{ | ||
AssertValidIndex(index); | ||
Debug.Assert(numberOfRows >= 1 && numberOfRows <= 0x0FFFFFFF); | ||
|
||
Span<byte> dataPos = _rentedBuffer.AsSpan(index + NumberOfRowsOffset); | ||
int current = MemoryMarshal.Read<int>(dataPos); | ||
|
||
// Persist the most significant nybble | ||
int value = (current & unchecked((int)0xF0000000)) | numberOfRows; | ||
MemoryMarshal.Write(dataPos, ref value); | ||
} | ||
|
||
internal void SetHasChildren(int index) | ||
{ | ||
AssertValidIndex(index); | ||
|
||
// The HasChildren bit is the most significant bit of "SizeOrLength" | ||
Span<byte> dataPos = _rentedBuffer.AsSpan(index + SizeOrLengthOffset); | ||
int current = MemoryMarshal.Read<int>(dataPos); | ||
|
||
int value = current | unchecked((int)0x80000000); | ||
MemoryMarshal.Write(dataPos, ref value); | ||
} | ||
|
||
internal int FindIndexOfFirstUnsetSizeOrLength(JsonTokenType lookupType) | ||
{ | ||
Debug.Assert(lookupType == JsonTokenType.StartObject || lookupType == JsonTokenType.StartArray); | ||
return FindOpenElement(lookupType); | ||
} | ||
|
||
private int FindOpenElement(JsonTokenType lookupType) | ||
{ | ||
Span<byte> data = _rentedBuffer.AsSpan(0, Length); | ||
|
||
for (int i = Length - DbRow.Size; i >= 0; i -= DbRow.Size) | ||
{ | ||
DbRow row = MemoryMarshal.Read<DbRow>(data.Slice(i)); | ||
|
||
if (row.IsUnknownSize && row.TokenType == lookupType) | ||
{ | ||
return i; | ||
} | ||
} | ||
|
||
// We should never reach here. | ||
Debug.Fail($"Unable to find expected {lookupType} token"); | ||
return -1; | ||
} | ||
|
||
internal void Get(int index, out DbRow row) | ||
{ | ||
AssertValidIndex(index); | ||
row = MemoryMarshal.Read<DbRow>(_rentedBuffer.AsSpan(index)); | ||
} | ||
|
||
internal int GetLocation() => MemoryMarshal.Read<int>(_rentedBuffer.AsSpan()); | ||
|
||
internal int GetLocation(int index) | ||
{ | ||
AssertValidIndex(index); | ||
return MemoryMarshal.Read<int>(_rentedBuffer.AsSpan()) & int.MaxValue; | ||
} | ||
|
||
internal int GetSizeOrLength(int index) | ||
{ | ||
AssertValidIndex(index); | ||
return MemoryMarshal.Read<int>(_rentedBuffer.AsSpan(index + SizeOrLengthOffset)) & int.MaxValue; | ||
} | ||
|
||
internal JsonTokenType GetJsonTokenType(int index = 0) | ||
{ | ||
AssertValidIndex(index); | ||
uint union = MemoryMarshal.Read<uint>(_rentedBuffer.AsSpan(index + NumberOfRowsOffset)); | ||
|
||
return (JsonTokenType)(union >> 28); | ||
} | ||
|
||
#if DIAGNOSTIC | ||
internal string PrintDatabase() | ||
{ | ||
StringBuilder sb = new StringBuilder(); | ||
sb.AppendLine("Index Offset SizeOrLen TokenType Child IPV NumRows"); | ||
Span<byte> data = _rentedBuffer; | ||
|
||
for (int i = 0; i < Length; i += DbRow.Size) | ||
{ | ||
DbRow record = MemoryMarshal.Read<DbRow>(data.Slice(i)); | ||
sb.Append($"{i:D6} {record.Location:D7} {record.SizeOrLength:D10} "); | ||
sb.Append(record.TokenType.ToString().PadRight(13)); | ||
sb.Append(record.HasChildren.ToString().PadRight(6)); | ||
sb.Append(record.IsPropertyValue.ToString().PadRight(6)); | ||
sb.AppendLine("" + record.NumberOfRows); | ||
} | ||
|
||
return sb.ToString(); | ||
} | ||
#endif | ||
} | ||
} | ||
} |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it going to be strange for folks to need to dispose an XxDocument? Is that required for other XxDocument in the framework? Is it required for any other JSON document-like types in other JSON libraries?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Usually the DOM APIs aren't IDisposable, but they have readers that are. So "no, but nearly". It seems like a natural consequence of the performance goals.
If it's "too strange for the surface area" then the next option that I see is to use generic List; the consequence of not disposing in the current model being that the document "stole" the array from the pool rather than rented it.