DescriptionDelay jumplist updates, filter redundant updates, and instrument updates.
Filter out redundant jumplist updates by ignoring provably pointless
updates and posting the update task with a delay to make more updates
redundant. A delay of 3500 ms was chosen because it should be short
enough to not noticeably affect the freshness of the jumplist, but
long enough to let more redundant requests arrive. A delay of 3500 ms
also guarantees (assuming a typical update time of ~500 ms) that the
browser will not spend a majority of its time updating the jumplist.
This change also adds Chrome tracing events to JumpList::PostRunUpdate
and JumpList::DeferredRunUpdate for testing and easier diagnosis of
update request storms.
There have been reports for years of excess updates to the Windows 7+
jumplist icons. One source of these problems is that
JumpList::PostRunUpdate may be called multiple times, queueing up a
long series of redundant updates.
A modest version of this was caught on my machine on the new-tab-page
where certain icons that fail to download trigger update requests
every time.
Another version of this has been inferred from ETW trace data from a
customer. This showed JumpList::RunUpdateOnFileThread being called
about 70 times over a 33 s period, with no gaps in-between. The
simplest explanation for this behavior is queueing of redundant
update requests. Note that this data shows that each JumpListUpdate
call consumes almost half a second, on a beefy machine with lots of
memory, many cores, and an SSD.
In that customer's pathological scenario we will now go from having
70+ jumplist updates to having just one, and we will be able to
better investigate the cause.
Other scenarios that have been reported include closing all tabs
during browser shutdown but I have not reproduced this.
It is worth separately investigating why JumpListUpdate is so
expensive (a FlushFileBuffers call for each icon is a problem) and
why these redundant calls are happening (NTP is one known cause,
the customer's calls appear to originate from syncer::GetSessionName)
but coalescing the redundant calls is still worthwhile.
While this change will not fix the related problem of ending up
with thousands of jumplist icon files it should reduce the number of
files created.
TEST=This change can be tested by going to chrome://tracing, clicking
Record, checking 'browser' only, and then clicking record. Then close a few
tabs in quick succession and wait five seconds before stopping the trace.
A search of the trace for JumpList should show a JumpList::PostRunUpdate
event for each tab that was closed, and a JumpList::DeferredRunUpdate for
the actual update that occurs ~3.5 s after the last
JumpList::PostRunUpdate. Bug 498987 tracks adding automated tests.
BUG=40407, 179576, 498987
Committed: https://crrev.com/c539ec64212e2a08e4b15c4ed29072e1d4f9fcab
Cr-Commit-Position: refs/heads/master@{#334015}
Patch Set 1 #Patch Set 2 : Adding delay to posting of jumplistupdate requests. #Patch Set 3 : Cleanup comments and delay amount. #
Total comments: 5
Patch Set 4 : Change implementation to use a OneShotTimer #Patch Set 5 : Fix some accumulated glitches #
Total comments: 6
Patch Set 6 : Fixing formatting nits. #Patch Set 7 : Add Chrome tracing to JumpList for testing and diagnostic purposes. #
Total comments: 1
Patch Set 8 : Changed from ui to browser category #Messages
Total messages: 28 (4 generated)
|