Right now, deleteProperty/removePropertyTransition causes a structure transition to uncacheable dictionary. Instead, we should allow it to transition to a new regular structure like adding a property does. This means that we have to: 1) Break the assumption that structure transition offsets increase monotonically 2) Add a new transition map and transition kind for deletes 3) Find some heuristic for when we should actually transition to uncacheable dictionary.
<rdar://problem/58706877>
Created attachment 388747 [details] Patch
WIP patch to see test results.
Known omissions: - the concurrent materialization thing - tests aren't complete: flatten, testing with inline cache Preliminary review, plus extra cases to test would be great.
Comment on attachment 388747 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=388747&action=review > Source/JavaScriptCore/runtime/Structure.cpp:577 > + return 0; nullptr
Created attachment 389033 [details] Patch
Created attachment 389083 [details] Patch
Created attachment 389307 [details] Patch
Created attachment 389399 [details] Patch
Created attachment 389597 [details] Patch
Comment on attachment 389597 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=389597&action=review r=me > Source/JavaScriptCore/runtime/PropertyMapHashTable.h:360 > +#ifndef NDEBUG Use #if ASSERT_ENABLED > Source/JavaScriptCore/runtime/PropertyMapHashTable.h:497 > +#ifndef NDEBUG Ditto. > Source/JavaScriptCore/runtime/Structure.cpp:564 > + if (Structure* existingTransition = structure->m_transitionTable.get(propertyName.uid(), attributes, false)) { Instead of passing `false`, let's pass, constexpr bool isAddition = false; if (Structure* existingTransition = structure->m_transitionTable.get(propertyName.uid(), attributes, isAddition)) { > Source/JavaScriptCore/runtime/Structure.cpp:581 > + for (auto* s = structure; s; s = s->previousID()) > + ++transitionCount; Let's stop this iteration when transitionCount exceeds s_maxTransitionLength > Source/JavaScriptCore/runtime/StructureTransitionTable.h:185 > + bool contains(UniquedStringImpl*, unsigned attributes, bool isAddition = true) const; > + Structure* get(UniquedStringImpl*, unsigned attributes, bool isAddition = true) const; I think specifying `isAddition` boolean in the caller side is easier to read instead of using default parameter.
Created attachment 389706 [details] Patch
Created attachment 389832 [details] Patch
Comment on attachment 389832 [details] Patch Clearing flags on attachment: 389832 Committed r255897: <https://trac.webkit.org/changeset/255897>
All reviewed patches have been landed. Closing bug.