Index PropertyKey, Object iterators and symbol support
#603
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
HalidOdat
added
enhancement
Codecov Report
@@ Coverage Diff @@
## master #603 +/- ##
==========================================
- Coverage 72.91% 72.49% -0.43%
==========================================
Files 178 179 +1
Lines 13226 13367 +141
==========================================
+ Hits 9644 9690 +46
- Misses 3582 3677 +95
Continue to review full report at Codecov.
|
Benchmark for cd40d51Click to view benchmark
|
Benchmark for b133ed9Click to view benchmark
|
Benchmark for 2db92d8Click to view benchmark
|
Benchmark for f81486dClick to view benchmark
|
Benchmark for 6f191f3Click to view benchmark
|
Benchmark for 8b5a0a6Click to view benchmark
|
Benchmark for 68ef7feClick to view benchmark
|
Benchmark for 3bae8e1Click to view benchmark
|
Benchmark for 57fbec8Click to view benchmark
|
Benchmark for 4cbfc08Click to view benchmark
|
HalidOdat
changed the title
PropertyKey and Object iteratorsPropertyKey, Object iterators and symbol support
Benchmark for 0a85013Click to view benchmark
|
Benchmark for 382a3a0Click to view benchmark
|
HalidOdat
force-pushed
the
index-property-key
branch
from
36f1321
to
7f054cb
Compare
Closed
Benchmark for 2eb34c2Click to view benchmark
|
Closed
Benchmark for 1a96533Click to view benchmark
|
Benchmark for 720ecd7Click to view benchmark
|
HalidOdat
force-pushed
the
index-property-key
branch
from
7d64602
to
548b295
Compare
Benchmark for daea89bClick to view benchmark
|
Benchmark for 591096bClick to view benchmark
|
Benchmark for 591096bClick to view benchmark
|
|
This is ready for review! :) Hmmm. The benchmarks are not showing the correct results! I did a local benchmark and there was a very small regression in 3 benchmarks (+5%) that is because we did not account for Symbols at all before. but the majority have a What do you think? @Razican |
Thanks! I will give this a look as soon as I can :)
Yep, we have to take the benchmarks with a grain of salt. I would say that if multiple benchmarks are showing the same, (which might be the case here), it might be because this is regressing in the particular case of the GitHub actions instance configuration. I will run the benchmarks locally just to make sure that there isn't something we are missing. In any case, I think it's fine to have some performance regressions when we add new stuff. I expect some performance regressions when we implement things such as async/await. |
HalidOdat
force-pushed
the
index-property-key
branch
from
ff818d4
to
8d16b21
Compare
Benchmark for 7adeeddClick to view benchmark
|
HalidOdat
force-pushed
the
index-property-key
branch
from
8d16b21
to
41d347f
Compare
Benchmark for 4d21eb2Click to view benchmark
|
HalidOdat
force-pushed
the
index-property-key
branch
from
41d347f
to
e821d8f
Compare
Benchmark for 3d77573Click to view benchmark
|
Lan2u
reviewed
Aug 18, 2020
Lan2u
approved these changes
Aug 18, 2020
Razican
reviewed
Aug 19, 2020
Benchmark for 5616890Click to view benchmark
|
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.
It changes the following:
itermethod to object to iterate over all (indexed, string and symbol) keys (PropertyKey) and value (Property)keysmethod to object to iterate over all (indexed, string and symbol) keys (PropertyKey)valuesmethod to object to iterate through all the (indexed, string and symbol) values (Property).symbol_propertiesmethod to object to iterate over symbol keys (RcSymbol) and value (Property)symbol_property_keysmethod to object to iterate over symbol keys (RcSymbol)symbol_property_valuesmethod to object to iterate over symbol values (Property)index_propertiesmethod to object to iterate over index keys (u32) and value (Property)index_property_keysmethod to object to iterate over index keys (u32)index_property_valuesmethod to object to iterate over index values (Property)string_propertiesmethod to object to iterate over string keys (RcString) and value (Property)string_property_keysmethod to object to iterate over string keys (RcString)string_property_valuesmethod to object to iterate over string values (Property)properties,properties_mut,symbol_propertiesandsymbol_properties_mut.Symbolindexing support..construct_symbol()for creating symbols.construct_object()for creating object with the prototype set.PropertyKey::Index(u32)This is done for a couple of reasons:u32and not asRcStrings (memory optimization)u32instead ofRcStrings).get_field(i.to_string())instead we should do.get_field(i)indexed_propertiesHash map with keyu32this is because an array index is from0to2^32 - 1(u32::MAX).IndexMapand when we need to iterate through all the elements (in order) we can do so without iteration through0tolength(this is what caused the test runner to freeze in Add ECMAScript test suite (test262) #567 withindexOfwith sparse elements). should make iteration very fast.RcString.Objectsymbol_propertiescontain keyRcSymbolinstead of just hash:Symboldescriptionwe need the symbol:Symbol.toPrimitive.description