Bug 215529 - Scrolling sync changes in r261985 regressed CPU usage by ~2 ms/s
Summary: Scrolling sync changes in r261985 regressed CPU usage by ~2 ms/s
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Simon Fraser (smfr)
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2020-08-14 18:13 PDT by Simon Fraser (smfr)
Modified: 2020-08-16 14:27 PDT (History)
11 users (show)

See Also:


Attachments
Patch (6.28 KB, patch)
2020-08-14 18:16 PDT, Simon Fraser (smfr)
ggaren: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Simon Fraser (smfr) 2020-08-14 18:13:54 PDT
Scrolling sync changes in r261985 regressed CPU usage by ~2 ms/s
Comment 1 Simon Fraser (smfr) 2020-08-14 18:16:47 PDT
Created attachment 406641 [details]
Patch
Comment 2 Simon Fraser (smfr) 2020-08-14 18:17:53 PDT
<rdar://problem/66866163>
Comment 3 Geoffrey Garen 2020-08-14 21:15:42 PDT
Comment on attachment 406641 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=406641&action=review

r=me

> Source/WebCore/page/scrolling/ScrollingTree.h:212
> +    bool isRecentlyActive();
> +    WEBCORE_EXPORT void setRecentlyActive();

Should we make these names more specific to wheel events?

isProcessingWheelEvent()
willProcessWheelEvent()

> Source/WebCore/page/scrolling/ScrollingTree.h:278
> +    Lock m_lastEventTimeMutex;
> +    MonotonicTime m_lastEventTime;

Should we make these names more specific to wheel events?

m_lastWheelEventTimeMutex
m_lastWheelEventTime
Comment 4 Simon Fraser (smfr) 2020-08-14 22:14:14 PDT
(In reply to Geoffrey Garen from comment #3)
> Comment on attachment 406641 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=406641&action=review
> 
> r=me
> 
> > Source/WebCore/page/scrolling/ScrollingTree.h:212
> > +    bool isRecentlyActive();
> > +    WEBCORE_EXPORT void setRecentlyActive();
> 
> Should we make these names more specific to wheel events?
> 
> isProcessingWheelEvent()
> willProcessWheelEvent()
> 
> > Source/WebCore/page/scrolling/ScrollingTree.h:278
> > +    Lock m_lastEventTimeMutex;
> > +    MonotonicTime m_lastEventTime;
> 
> Should we make these names more specific to wheel events?
> 
> m_lastWheelEventTimeMutex
> m_lastWheelEventTime

Perhaps, though the ScrollingThread exists only to process wheel events, so I don't feel the need to qualify further.
Comment 5 Simon Fraser (smfr) 2020-08-16 14:27:53 PDT
https://trac.webkit.org/changeset/265743/webkit