I noticed while hacking on Structure that we sometimes create rare data for Structure clones where only the previousID pointer is set. This is silly since that pointer could then be stored in the m_previousOrRareData field.
Created attachment 191900 [details] Proposed patch
Comment on attachment 191900 [details] Proposed patch r=me. Might need a rebase. And we don't need to clone StructureRareData if we have an m_enumerationCache because it's just a cache, correct?
(In reply to comment #2) > (From update of attachment 191900 [details]) > r=me. Might need a rebase. And we don't need to clone StructureRareData if we have an m_enumerationCache because it's just a cache, correct? Correct, enumerationCache is just a cache. On that note, objectToStringValue is also just a cache for objectProtoFuncToString(). We could actually skip cloning StructureRareData entirely (given the members we have today.) I only kept the objectToStringValue cloning because we were already doing it, so I assumed it was a more expensive operation. What do you think? Should we just skip cloning the rare data entirely instead?
> What do you think? Should we just skip cloning the rare data entirely instead? You're right that cloning right now is silly because both things are just a cache, but I think we'll want to clone some things in the near future as we add more things to StructureRareData, so deleting all of the cloning-related code might be premature. Another idea might be to make the StructureRareData just be things that are cacheable and to never clone them. I see a couple things that look like they're just caches (e.g. m_cachedPrototypeChain). We could then also do the "throw away for the cacheable stuff during GC" trick. I guess for now maybe add a function to StructureRareData along the lines of "bool needsCloning()" that just returns false right and then check that method when determining whether or not to clone. That way we don't have to make a decision right now as to the direction of StructureRareData.
Created attachment 193343 [details] Patch for landing Incorporated Mark's suggestion to let StructureRareData::needsCloning() decide whether to clone rare data. It always returns false, for now.
Comment on attachment 193343 [details] Patch for landing Clearing flags on attachment: 193343 Committed r145947: <http://trac.webkit.org/changeset/145947>
All reviewed patches have been landed. Closing bug.