1357801 - aRequest.URI and aRequest.originalURI take 12ms of running time during Kraken

Status: RESOLVED FIXED

(Firefox :: Tabbed Browser, enhancement)

Description

[Jared Wein [:jaws] (please needinfo? me)]

See https://perf-html.io/public/2b9073fb8116f1d7508b71166f388fdb545f73b2/calltree/?jsOnly&range=54.1559_79.8503~58.8305_79.8503&search=get%20&thread=0 and filter on 'get '.

We already call these two getters earlier and can pass their instances to the calling functions instead of having the calling functions request them again.

Comment 1

[Jared Wein [:jaws] (please needinfo? me)]

Visit https://krakenbenchmark.mozilla.org/ to run the test.

Comment 2

[:Gijs (he/him)]

Are these data: URIs or something? And, if we get these on channels, isn't this a networking bug?

Comment 3

[Jared Wein [:jaws] (please needinfo? me)]

Attachment: kraken-uri-originalURI.txt

I ran Kraken with logging inside of onStateChange and _shouldShowProgress and these are the URIs that are being loaded. In the logging, the first URI is the request.URI field, and the second one is request.originalURI.

Most of them are on the wyciwyg schema with a network URL [1], and the others are for about:blank.

[1] wyciwyg://152/https://krakenbenchmark.mozilla.org/kraken-1.1/driver.html for example, when running test 152.

Comment 4

[Jared Wein [:jaws] (please needinfo? me)]

Attachment: Bug 1357801 - Run Kraken in a chromeless browser window so as to remove any noise from UI changes.

Review commit: https://reviewboard.mozilla.org/r/151498/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/151498/

Comment 5

[Mike Conley (:mconley) (:⚙️)]

Comment on attachment 8880113 [details]
Bug 1357801 - Run Kraken in a chromeless browser window so as to remove any noise from UI changes.

https://reviewboard.mozilla.org/r/151498/#review156426

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/84ec2922201d
Run Kraken in a chromeless browser window so as to remove any noise from UI changes. r=mconley

Comment 7

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/84ec2922201d

Comment 8

[:Gijs (he/him)]

Did we consider doing what comment #0 suggests in addition to decoupling kraken from the browser UI? Seems like that might still be worthwhile...

Comment 9

[Jared Wein [:jaws] (please needinfo? me)]

Yes, I considered it. The 12ms of time shown is more evident in the case of Kraken which hit this code about 200 times (if my memory serves me correctly). Doing what comment #0 suggested didn't make a change that was an order of magnitude better. It maybe cut it down from 12ms to 7-8ms, which to me wasn't worth the change in complexity. The real bug here was that we were getting regression notices for use cases that are not representative of normal browser usage.