Summary: | [results.webkit.org Timeline] Performance improvements | ||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Zhifei Fang <zhifei_fang> | ||||||||||||||||
Component: | Tools / Tests | Assignee: | Nobody <webkit-unassigned> | ||||||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||||||
Severity: | Normal | CC: | commit-queue, jbedard, webkit-bug-importer, zhifei_fang | ||||||||||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||||||||||
Version: | Safari Technology Preview | ||||||||||||||||||
Hardware: | Unspecified | ||||||||||||||||||
OS: | Unspecified | ||||||||||||||||||
Attachments: |
|
Description
Zhifei Fang
2019-08-02 14:35:37 PDT
Created attachment 375458 [details]
Patch
Comment on attachment 375458 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=375458&action=review > Tools/ChangeLog:8 > + * resultsdbpy/resultsdbpy/view/static/library/js/Ref.js: We need some comments here. Created attachment 375465 [details]
Patch
Created attachment 375474 [details]
Patch
Comment on attachment 375465 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=375465&action=review > Tools/resultsdbpy/resultsdbpy/view/static/library/js/components/TimelineComponents.js:620 > + console.log(stateDiff); Do you intend to land this logging statement? Created attachment 375583 [details]
Patch
Comment on attachment 375583 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=375583&action=review Along with fixing a few pretty clear correctness bugs, this patch has a HUGE impact on memory usage of timelines, particularly large ones. I've observed about 5x in some cases, probably 2 or 3x on average. > Tools/resultsdbpy/resultsdbpy/view/static/library/js/Utils.js:69 > +// Check more about Intersection observer API I would put this in one-line: // Uses intersection observer: <https://developer.mozilla.org/en-US/docs/Web/API/Intersection_Observer_API> > Tools/resultsdbpy/resultsdbpy/view/static/library/js/components/TimelineComponents.js:380 > + // We do nothing for those off screen ones Comment is duplicating what the code says, just remove it. How would the element be on screen but the stateDiff not? > Tools/resultsdbpy/resultsdbpy/view/static/library/js/components/TimelineComponents.js:763 > + // In case of modification white rendering Nit: while rendering (In reply to Jonathan Bedard from comment #7) > Comment on attachment 375583 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=375583&action=review > > Along with fixing a few pretty clear correctness bugs, this patch has a HUGE > impact on memory usage of timelines, particularly large ones. I've observed > about 5x in some cases, probably 2 or 3x on average. > > > Tools/resultsdbpy/resultsdbpy/view/static/library/js/Utils.js:69 > > +// Check more about Intersection observer API > > I would put this in one-line: > > // Uses intersection observer: > <https://developer.mozilla.org/en-US/docs/Web/API/Intersection_Observer_API> > > > Tools/resultsdbpy/resultsdbpy/view/static/library/js/components/TimelineComponents.js:380 > > + // We do nothing for those off screen ones > > Comment is duplicating what the code says, just remove it. > > How would the element be on screen but the stateDiff not? stateDiff is a diff, when we already set onScreen to true, it will always be true in the state, unless we want to set it to false. > > > Tools/resultsdbpy/resultsdbpy/view/static/library/js/components/TimelineComponents.js:763 > > + // In case of modification white rendering > > Nit: while rendering Just change the title, this patch now include multiple performance improvements for the canvas timeline. 1. Unhook the scroll event when a series/axis have been removed from the container 2. Fix the axis's cache date structure out of sync. 3. Use position:sticky to reduce the blink when update the transform 4. Use intersection observer to detect if the canvas on screen or not, if a canvas is not on the screen, we do nothing, this will eliminate render requests we send out. Created attachment 375592 [details]
Patch
Comment on attachment 375592 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=375592&action=review > Tools/ChangeLog:10 > + Reviewed by NOBODY (OOPS!). WebKit does: <change title> <bug URL> Reviewed by <Reviewer>. <change details> ----- So can we move the numbered list under the 'Reviewed by' line? Created attachment 375630 [details]
Patch
Created attachment 375631 [details]
Patch
Comment on attachment 375631 [details] Patch Clearing flags on attachment: 375631 Committed r248305: <https://trac.webkit.org/changeset/248305> All reviewed patches have been landed. Closing bug. |