Optimize PointerCaptureController::elementWasRemoved()
Created attachment 419102 [details] Patch
Comment on attachment 419102 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=419102&action=review > Source/WebCore/page/PointerCaptureController.cpp:178 > +void PointerCaptureController::updateHaveAnyCapturingElement() Do we need to eagerly update this or can we compute it lazily? e.g. use Optional<bool> for m_haveAnyCapturingElement and then haveAnyCapturingElement() inlined like haveAnyCapturingElement() { return m_haveAnyCapturingElement ? *m_haveAnyCapturingElement : haveAnyCapturingElementSlowCase(); } > Source/WebCore/page/PointerCaptureController.cpp:180 > + auto haveAnyCapturingElement = [&] { Do we really need to use lambda here? I find that setting m_haveAnyCapturingElement = false and breaking when the condition is hit would be easier to read. Alternatively, can we just use anyOf in Algorithms.h so that: m_haveAnyCapturingElement = WTF::anyOf(m_activePointerIdsToCapturingData.values(), [&] { ~ }) > Source/WebCore/page/PointerCaptureController.cpp:183 > + if (capturingData.pendingTargetOverride || capturingData.targetOverride Can we add a helper function on CapturingData like CapturingData::hasCapturingElement()?
(In reply to Ryosuke Niwa from comment #2) > Comment on attachment 419102 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=419102&action=review > > > Source/WebCore/page/PointerCaptureController.cpp:178 > > +void PointerCaptureController::updateHaveAnyCapturingElement() > > Do we need to eagerly update this or can we compute it lazily? > e.g. use Optional<bool> for m_haveAnyCapturingElement > and then haveAnyCapturingElement() inlined like haveAnyCapturingElement() { > return m_haveAnyCapturingElement ? *m_haveAnyCapturingElement : > haveAnyCapturingElementSlowCase(); } I think it's better to do the work eagerly in the functions that start/stop capture, since the no-capture case is massively more common than the capturing case. > > > Source/WebCore/page/PointerCaptureController.cpp:180 > > + auto haveAnyCapturingElement = [&] { > > Do we really need to use lambda here? > I find that setting m_haveAnyCapturingElement = false and breaking when the > condition is hit would be easier to read. > Alternatively, can we just use anyOf in Algorithms.h so that: > m_haveAnyCapturingElement = > WTF::anyOf(m_activePointerIdsToCapturingData.values(), [&] { ~ }) I like the lambda. but I can change it. > > Source/WebCore/page/PointerCaptureController.cpp:183 > > + if (capturingData.pendingTargetOverride || capturingData.targetOverride > > Can we add a helper function on CapturingData like > CapturingData::hasCapturingElement()? Will do.
(In reply to Simon Fraser (smfr) from comment #3) > (In reply to Ryosuke Niwa from comment #2) > > Comment on attachment 419102 [details] > > Patch > > > > View in context: > > https://bugs.webkit.org/attachment.cgi?id=419102&action=review > > > > > Source/WebCore/page/PointerCaptureController.cpp:178 > > > +void PointerCaptureController::updateHaveAnyCapturingElement() > > > > Do we need to eagerly update this or can we compute it lazily? > > e.g. use Optional<bool> for m_haveAnyCapturingElement > > and then haveAnyCapturingElement() inlined like haveAnyCapturingElement() { > > return m_haveAnyCapturingElement ? *m_haveAnyCapturingElement : > > haveAnyCapturingElementSlowCase(); } > > I think it's better to do the work eagerly in the functions that start/stop > capture, since the no-capture case is massively more common than the > capturing case. Ok. > > > Source/WebCore/page/PointerCaptureController.cpp:180 > > > + auto haveAnyCapturingElement = [&] { > > > > Do we really need to use lambda here? > > I find that setting m_haveAnyCapturingElement = false and breaking when the > > condition is hit would be easier to read. > > Alternatively, can we just use anyOf in Algorithms.h so that: > > m_haveAnyCapturingElement = > > WTF::anyOf(m_activePointerIdsToCapturingData.values(), [&] { ~ }) > > I like the lambda. but I can change it. Please use anyOf then.
https://trac.webkit.org/changeset/272331/webkit
<rdar://problem/73939380>