1309510 - Investigate performance differences between old and new icon code

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: Favicon Handling, defect)

Description

[Sebastian Kaspari (:sebastian; :pocmo)]

In bug 1300490 we saw a performance regression after landing the new icon code.

* Only nytimes spiked but not the twitter test - Why?
* The old and new icon code both run asynchronously, so it shouldn't affect page loading too much. Why does it?
* To avoid performance issues we intentionally load icons only *after* the page has loaded. So why does it affect our performance tests?

To answer those questions and debug the performance regression I'll have to look closer at the code and how all components interact - and at what points in time we log the throbber messages for the performance measurements.

Comment 1

[Sebastian Kaspari (:sebastian; :pocmo)]

No change (mozilla-central):
https://treeherder.mozilla.org/#/jobs?repo=try&revision=495c97047fa2

Icon loading disabled:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=79674e20e7e1

Comment 2

[Sebastian Kaspari (:sebastian; :pocmo)]

Disabled loading and building icon requests:
 https://treeherder.mozilla.org/#/jobs?repo=try&revision=00e33c2154df88d60748608f0276a0b30e07d193

Comment 3

[Sebastian Kaspari (:sebastian; :pocmo)]

Attachment: local-nytimes.png

Comment 4

[Sebastian Kaspari (:sebastian; :pocmo)]

Attachment: remote-nytimes.png

Comment 5

[Sebastian Kaspari (:sebastian; :pocmo)]

I measured the current code (no change), loading disabled and code completely removed for local-nytimes and remote-nytimes on try.

I couldn't find any noticeable difference. In fact often the tests with no code change ran faster - again pointing into the direction that this doesn't affect page loading much.

In addition to that I looked at the code and the icon load is triggered a little bit before logging the throbber messages. They listen to different events and it doesn't make sense to align them so that icon loading is happening after the throbber messages for performance measurements.

Right now there doesn't seem to be a measurable improvement to make.