Closed
Bug 1406312
Opened 7 years ago
Closed 7 years ago
Waterfall's timingBoxes is slow because of its l10n calls
Categories
(DevTools :: Netmonitor, enhancement, P3)
DevTools
Netmonitor
Tracking
(firefox57 fix-optional, firefox58 fixed)
RESOLVED
FIXED
Firefox 58
Tracking | Status | |
---|---|---|
firefox57 | --- | fix-optional |
firefox58 | --- | fixed |
People
(Reporter: ochameau, Assigned: abhinav.koppula)
References
Details
Attachments
(2 files)
https://perfht.ml/2xmarmo * Select "Filer: JS Only" * Search for timingBoxes * Expand the call tree until ending up on "timingBoxes" * See getFormatStr taking 41ms and timingBoxes 74ms This bug is similar to bug 1404130. Calling L10N function is slow. bug 1406311 is about making them faster, but may be we could make tooltip computation lazy? May be it can be computed only when we hover the element with the tooltip?
Updated•7 years ago
|
status-firefox57:
--- → fix-optional
Priority: -- → P3
Comment 1•7 years ago
|
||
Thanks for the profiling, from the screenshot it looks L10N getFormatString cause most of time on render the waterfall.
Assignee | ||
Comment 2•7 years ago
|
||
Hi Fred, Can I try this one?
Comment hidden (mozreview-request) |
Assignee | ||
Comment 4•7 years ago
|
||
Hi Fred, Alexandre, I have given a shot at this issue. Would this help in performance improvement?
Reporter | ||
Comment 5•7 years ago
|
||
(In reply to Abhinav Koppula from comment #4) > Hi Fred, Alexandre, > I have given a shot at this issue. Would this help in performance > improvement? Yes! I see "timingBoxes" go from 117ms down to 53ms. getFormatStr still takes 25ms, so is still significant, but hopefully julian is going to improve it via bug 1406311.
Reporter | ||
Updated•7 years ago
|
Assignee: nobody → abhinav.koppula
Comment 6•7 years ago
|
||
mozreview-review |
Comment on attachment 8916525 [details] Bug 1406312 - Lazy loading of tooltip text on hover in Waterfall Timing boxes. https://reviewboard.mozilla.org/r/187658/#review192956 Looks good! Just need to address some code consitency issue. And L10N might the reason let resize feel slow, since we need do all string formatting again and again during resize... For the left L10N.getFormatStr. Chrome put the time label in a separate time column, we could have that too. ::: devtools/client/netmonitor/src/components/request-list-column-waterfall.js:42 (Diff revision 1) > this.props.firstRequestStartedMillis !== nextProps.firstRequestStartedMillis; > }, > > render() { > let { firstRequestStartedMillis, item, onWaterfallMouseDown } = this.props; > - const { boxes, tooltip } = timingBoxes(item); > + const { boxes } = timingBoxes(item); Since we have one return value now, can do `const boxes = timingBoxes(item);` ::: devtools/client/netmonitor/src/components/request-list-column-waterfall.js:50 (Diff revision 1) > - div({ > + div({ > + className: "requests-list-column requests-list-waterfall", > + onMouseOver: function ({target}) { > + if (!target.title) { > + let tooltipTitle = timingTooltip(item); > + target.title = tooltipTitle; can do `target.title = timingTooltip(item);` without define `tooltipTitle` variable ::: devtools/client/netmonitor/src/components/request-list-column-waterfall.js:98 (Diff revision 1) > let { eventTimings, fromCache, fromServiceWorker, totalTime } = item; > let boxes = []; > - let tooltip = []; > > if (fromCache || fromServiceWorker) { > - return { boxes, tooltip }; > + return { boxes }; `return boxes;` ::: devtools/client/netmonitor/src/components/request-list-column-waterfall.js:131 (Diff revision 1) > }, title) > ); > - tooltip.push(L10N.getFormatStr("netmonitor.waterfall.tooltip.total", totalTime)); > } > > return { return boxes;
Attachment #8916525 -
Flags: review?(gasolin) → review+
Comment hidden (mozreview-request) |
Assignee | ||
Comment 8•7 years ago
|
||
Hi Fred, Thanks for the review. Have made the suggested changes. Can you check once?
Comment 9•7 years ago
|
||
Thanks for update, test green, I'll try to land it.
Comment 10•7 years ago
|
||
Pushed by flin@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/e92ca171760d Lazy loading of tooltip text on hover in Waterfall Timing boxes. r=gasolin
Comment 11•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/e92ca171760d
Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox58:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 58
Updated•6 years ago
|
Product: Firefox → DevTools
You need to log in
before you can comment on or make changes to this bug.
Description
•