It was erroneously calling didFindOpaqueRoot() all the time even when the root is not found. rdar://78208014
Created attachment 429126 [details] proposed patch.
Comment on attachment 429126 [details] proposed patch. View in context: https://bugs.webkit.org/attachment.cgi?id=429126&action=review > Source/JavaScriptCore/heap/AbstractSlotVisitorInlines.h:136 > + if (UNLIKELY(found && m_needsExtraOpaqueRootHandling)) { Is there a reason to check found first and m_needsExtraOpaqueRootHandling second? If anything, I would expect the entire check of m_opaqueRoots.contains(ptr) to be only done after checking m_needsExtraOpaqueRootHandling, especially if it is UNLIKELY.
(In reply to Robin Morisset from comment #2) > Comment on attachment 429126 [details] > proposed patch. > > View in context: > https://bugs.webkit.org/attachment.cgi?id=429126&action=review > > > Source/JavaScriptCore/heap/AbstractSlotVisitorInlines.h:136 > > + if (UNLIKELY(found && m_needsExtraOpaqueRootHandling)) { > > Is there a reason to check found first and m_needsExtraOpaqueRootHandling > second? > > If anything, I would expect the entire check of m_opaqueRoots.contains(ptr) > to be only done after checking m_needsExtraOpaqueRootHandling, especially if > it is UNLIKELY. I check found first because found is already in memory. If not found, we save on triggering a load of m_needsExtraOpaqueRootHandling.
Comment on attachment 429126 [details] proposed patch. View in context: https://bugs.webkit.org/attachment.cgi?id=429126&action=review r=me >>> Source/JavaScriptCore/heap/AbstractSlotVisitorInlines.h:136 >>> + if (UNLIKELY(found && m_needsExtraOpaqueRootHandling)) { >> >> Is there a reason to check found first and m_needsExtraOpaqueRootHandling second? >> >> If anything, I would expect the entire check of m_opaqueRoots.contains(ptr) to be only done after checking m_needsExtraOpaqueRootHandling, especially if it is UNLIKELY. > > I check found first because found is already in memory. If not found, we save on triggering a load of m_needsExtraOpaqueRootHandling. I had missed that we eventually return found and so we need to compute it even if !m_needsExtraOpaqueRootHandling.
Comment on attachment 429126 [details] proposed patch. Thanks for the review. Landing now.
Committed r277773 (237934@main): <https://commits.webkit.org/237934@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 429126 [details].