Object members stored in std::multimap #1485
Merged
Conversation
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
|
Some numbers: |
ylavic
force-pushed
the
MemberMap
branch
4 times, most recently
from
50d0bd8
to
5c454a0
Compare
|
This seems promising but it really needs time to review. |
ylavic
force-pushed
the
MemberMap
branch
3 times, most recently
from
a778622
to
514af25
Compare
ylavic
force-pushed
the
MemberMap
branch
4 times, most recently
from
e4ab391
to
70815d8
Compare
This was referenced Apr 28, 2019
Merged
ylavic
force-pushed
the
MemberMap
branch
6 times, most recently
from
d5498df
to
858a9b3
Compare
ylavic
force-pushed
the
MemberMap
branch
3 times, most recently
from
dcfdc22
to
a8cf249
Compare
|
@miloyip this PR needs 7546eb2 from #1503 to pass (see https://travis-ci.org/github/Tencent/rapidjson/jobs/765633543#L2086 for instance), the allocator cannot be destroyed before the Values using it. I'll merge PR #1503 here until it's in master.. |
|
Actually the whole PR #1503 is needed (not only 7546eb2, but also c033292), otherwise with Currently |
ylavic
force-pushed
the
MemberMap
branch
4 times, most recently
from
c090ee8
to
a2343c6
Compare
…ers.
When RAPIDJSON_USE_MEMBERSMAP is defined, an object Value will store
its members in an (re)allocated array of Members like before, but also
in an std::multimap<GenericValue::Data,SizeType> where the key and value
reference the corresponding Member by its Data and index in the array,
respectively, and in a relocatable manner.
The layout of the members map/array is now:
{multimap*}<>{capacity}<>{Member[capacity]}<>{multimap::iterator[capacity]}
where <> stands for the RAPIDJSON_ALIGN-ment of each part, if needed.
This layout needs to be reallocated when the current capacity is
exhausted, which requires to take care of the multimap and its iterators
explicitely. The multimap is allocated separately and only its pointer is
saved in this layout, so it can easily be restored in its new position.
As for the old/alive iterators, they must move to their new offset according
to the new capacity.
With this in place, it's immediate to get the multimap::iterator from a
MemberIterator and vice versa, thus the same complexity applies for the
operations with MemberIterator or MapIterator.
For FindMember() and RemoveMember(), the complexity drops from O(n) to
the multimap/rbtree's O(log n).
For EraseMember() it drops from O(n-m) to O((log n)-m), m representing
the move/copy of the trailing members.
For AddMember() though, the complexity grows from O(1) to O(log n) due to
the insertion in the multimap too.
Consequently parsing will be slower, up to ~20% measured in perftests on
my laptop (since it's mainly composed of insertions). But later work on
the Document (usually the goal of parsing...) will be much faster; the
new DocumentFind perftest included in this commit is 8 times faster with
RAPIDJSON_USE_MEMBERSMAP (still on my laptop). Overall the tests are 4%
slower (mainly composed of parsing), and notably 15% slower for schemas
parsing/validation (which supposedly comes from the larger JSON files
parsing, still). As a side note, when RAPIDJSON_USE_MEMBERSMAP is not
defined, this commit does nothing (same results for perftest with regard
to previous versions).
Finally, the multimap is allocated and constructed using StdAllocator,
so they will use the same Allocator than for any other Value allocation,
and thus will benefit from the same performance/safety/security/whatever
provided by the user given Allocator.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Set RAPIDJSON_USE_MEMBERSMAP to use a (std::multi)map for object members.
When
RAPIDJSON_USE_MEMBERSMAPis defined, an objectValuewill storeits members in an (re)allocated array of Members like before, but also
in an
std::multimap<GenericValue::Data,SizeType>where the key and valuereference the corresponding
Memberby its Data and index in the array,respectively, and in a relocatable manner.
The layout of the members map/array is now:
{multimap*}<>{capacity}<>{Member[capacity]}<>{multimap::iterator[capacity]}where
<>stands for theRAPIDJSON_ALIGN-ment of each part, if needed.This layout needs to be reallocated when the current capacity is
exhausted, which requires to take care of the
multimapand its iteratorsexplicitely. The
multimapis allocated separately and only its pointer issaved in this layout, so it can easily be restored in its new position.
As for the old/alive iterators, they must move to their new offset according
to the new capacity.
With this in place, it's immediate to get the
multimap::iteratorfrom aMemberIterator and vice versa, thus the same complexity applies for the
operations with MemberIterator or MapIterator.
For
FindMember()andRemoveMember(), the complexity drops fromO(n)tothe
multimap/rbtree'sO(log n).For
EraseMember()it drops fromO(n-m)toO((log n)-m),mrepresentingthe move/copy of the trailing members.
For
AddMember()though, the complexity grows fromO(1)toO(log n)due tothe insertion in the
multimaptoo.Consequently parsing will be slower, up to ~20% measured in perftests on
my laptop (since it's mainly composed of insertions). But later work on
the
Document(usually the goal of parsing...) will be much faster; thenew
DocumentFindperftest included in this commit is 8 times faster withRAPIDJSON_USE_MEMBERSMAP(still on my laptop). Overall the tests are 4%slower (mainly composed of parsing), and notably 15% slower for schemas
parsing/validation (which supposedly comes from the larger JSON files
parsing, still). As a side note, when
RAPIDJSON_USE_MEMBERSMAPis notdefined, this commit does nothing (same results for perftest with regard
to previous versions).
Finally, the
multimapis allocated and constructed using StdAllocator,so they will use the same Allocator than for any other
Valueallocation,and thus will benefit from the same performance/safety/security/whatever
provided by the user given Allocator.