Alexandre Poirot [:ochameau] Reporter
	Description • 7 years ago

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?

	gasolin@mozilla.com
	Comment 1 • 7 years ago

Thanks for the profiling, from the screenshot it looks L10N getFormatString cause most of time on render the waterfall.

	Abhinav Koppula Assignee
	Comment 2 • 7 years ago

Hi Fred,
Can I try this one?

	Abhinav Koppula Assignee
	Comment 4 • 7 years ago

Hi Fred, Alexandre,
I have given a shot at this issue. Would this help in performance improvement?

	Alexandre Poirot [:ochameau] 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.

	gasolin@mozilla.com
	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;

	Abhinav Koppula Assignee
	Comment 8 • 7 years ago

Hi Fred,
Thanks for the review. Have made the suggested changes.
Can you check once?

	gasolin@mozilla.com
	Comment 9 • 7 years ago

Thanks for update, test green, I'll try to land it.

	Pulsebot
	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

	Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)
	Comment 11 • 7 years ago
bugherder

https://hg.mozilla.org/mozilla-central/rev/e92ca171760d