Enable ES6IsConcatSpreadable under experimental #1198
Conversation
| BOOL result0, result1; | ||
|
|
||
| public: | ||
| IsConcatSpreadableCache() : |
There was a problem hiding this comment.
Initialized these as initialization list.
|
@Microsoft/chakra-es any other questions or comments? |
|
Did you respond to my comment about handling side-effects? -- Paul From: suwcmailto:notifications@github.com @Microsoft/chakra-eshttps://github.com/orgs/Microsoft/teams/chakra-es any more questions? — |
|
Quote "I don't see the part of the change that clears the cache if there are side-effects of reading isConcatSpreadable. Is that coming later? Or did I miss it?" To address functionality gap (namely two variable of same Type; search for “two arrays that may share the same type” in new unit tests) found with caching, the condition for caching isConcatSpreadable result has been narrowed down to: No user-defined [@@isConcatSpreadable] property Therefore there is no longer need to check if there are side-effects. In reply to: 228463392 [](ancestors = 228463392) |
| } | ||
| }, | ||
| { | ||
| name: "[@@isConcatSpreadable] getter altering binding", |
There was a problem hiding this comment.
is there a test which alters the length when you access the isConcatSpreadable?
There was a problem hiding this comment.
| @@ -964,6 +964,72 @@ namespace Js | |||
| void Unregister(ScriptContext * scriptContext); | |||
| }; | |||
|
|
|||
| // Two-entry Type-indexed circular cache | |||
There was a problem hiding this comment.
Did you do any experimentation with different cache sizes?
There was a problem hiding this comment.
Yes. Tried 1-, 2-, and 3-entry caches. From 1 to 2 saw significant perf gain; from 2 to 3 no gain. Therefore chose 2-entry for simplicity.
In reply to: 68804491 [](ancestors = 68804491)
Enable ES6IsConcatSpreadable under experimental flag. Optimize fastpath for cases where [@@isConcatSpreadable] is undefined by caching in ThreadContext. Add cache invalidations when users add [@@isConcatSpreadable] properties. Add unit tests to cover more extended usage scenarios.
|
@agarwal-sandeep review the changes in ConfigFlagsList.h? |
|
ConfigFlagsList.h change looks good. |
Merge pull request #1198 from suwc:build/suwc/buddy Enable ES6IsConcatSpreadable under experimental flag. Optimize fastpath for cases where [@@isConcatSpreadable] is undefined by caching in ThreadContext. Add cache invalidations when users add [@@isConcatSpreadable] properties. Add unit tests to cover more extended usage scenarios.
Enable ES6IsConcatSpreadable under experimental flag.
Optimize fastpath for cases where [@@isConcatSpreadable] is undefined by caching in ThreadContext.
Add cache invalidations when users add [@@isConcatSpreadable] properties.
Add unit tests to cover more extended usage scenarios.