Closed Bug 1461339 Opened 5 years ago Closed 4 years ago
Don't eagerly sparsify dense elements in Native
When a dense element is redefined to a non-writable property in NativeDefineProperty (and the property descriptor has no value field!), the element is first changed to a sparse property and then AddOrChangeProperty is called which updates the property attributes. Similar code is present when redefining a dense element to an accessor property . I don't think it's necessary to sparsify the dense element here, given that, for example, we don't do it when the redefining a dense element to a non-configurable property.  https://searchfox.org/mozilla-central/rev/a85db9e29eb3f022dbaf8b9a6390ecbacf51e7dd/js/src/vm/NativeObject.cpp#1807-1812  https://searchfox.org/mozilla-central/rev/a85db9e29eb3f022dbaf8b9a6390ecbacf51e7dd/js/src/vm/NativeObject.cpp#1789-1794
That means we can remove sparsifyDenseElement? Cool!
(In reply to Jan de Mooij [:jandem] from comment #1) > That means we can remove sparsifyDenseElement? Cool! Exactly! :-) In addition to the sparsifyDenseElement removal, the patch: - Changes |NativeObject::removeDenseElementForSparseIndex()| to a non-static method to better match its sibling methods in NativeObject. - Adds an assertion to |removeDenseElementForSparseIndex()| to ensure a sparse property is actually present. (The assertion is not needed for correctness, it is only intended to help to understand the code.) - And changes |removeDenseElementForSparseIndex()| to call |setDenseElement()| instead of |setDenseElementUnchecked()|, because |removeDenseElementForSparseIndex()| is now only called in situations where the elements are definitely not frozen. - |setDenseElementUnchecked()| is now only called in ||setDenseElement()|, so let's inline it and then remove |setDenseElementUnchecked()|. And do the same for |setDenseInitializedLengthUnchecked()|.
Attachment #8975492 - Flags: review?(jdemooij)
Comment on attachment 8975492 [details] [diff] [review] bug1461339.patch Review of attachment 8975492 [details] [diff] [review]: ----------------------------------------------------------------- \o/
Attachment #8975492 - Flags: review?(jdemooij) → review+
Pushed by firstname.lastname@example.org: https://hg.mozilla.org/integration/mozilla-inbound/rev/1910489ee839 Don't sparsify dense elements eagerly in NativeDefineProperty. r=jandem
You need to log in before you can comment on or make changes to this bug.