Last Comment Bug 687081 - Add telemetry for time to find a cache entry
: Add telemetry for time to find a cache entry
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Networking: Cache (show other bugs)
: unspecified
: All All
-- normal (vote)
: mozilla10
Assigned To: OFFLINE UNTIL 27 FEB 2017 Nicholas Hurley [:nwgh][:hurley] (also hurley@todesschaf.org)
:
: Patrick McManus [:mcmanus]
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2011-09-16 09:29 PDT by OFFLINE UNTIL 27 FEB 2017 Nicholas Hurley [:nwgh][:hurley] (also hurley@todesschaf.org)
Modified: 2011-10-31 11:21 PDT (History)
1 user (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
timing patch (12.48 KB, patch)
2011-09-28 10:45 PDT, OFFLINE UNTIL 27 FEB 2017 Nicholas Hurley [:nwgh][:hurley] (also hurley@todesschaf.org)
jduell.mcbugs: review+
Details | Diff | Splinter Review
updated patch (6.26 KB, patch)
2011-10-28 11:28 PDT, OFFLINE UNTIL 27 FEB 2017 Nicholas Hurley [:nwgh][:hurley] (also hurley@todesschaf.org)
hurley: review+
Details | Diff | Splinter Review

Description User image OFFLINE UNTIL 27 FEB 2017 Nicholas Hurley [:nwgh][:hurley] (also hurley@todesschaf.org) 2011-09-16 09:29:01 PDT
Want to know how long we're spending searching the cache in total, as well as how long we're spending on each of memory/disk/offline. Could try to correlate with cache miss somehow, too?
Comment 1 User image OFFLINE UNTIL 27 FEB 2017 Nicholas Hurley [:nwgh][:hurley] (also hurley@todesschaf.org) 2011-09-28 10:45:46 PDT
Created attachment 563109 [details] [diff] [review]
timing patch

Patch that measures time to lookup cache entries in us (originally did this with telemetry autotimers, but since those are in ms the results weren't very interesting to look at)
Comment 2 User image Jason Duell [:jduell] (needinfo me) 2011-10-24 22:02:58 PDT
Comment on attachment 563109 [details] [diff] [review]
timing patch

Review of attachment 563109 [details] [diff] [review]:
-----------------------------------------------------------------

+r so long as you change all the places where you call telemetry code at every return point in the function to instead just use a Telemetry::AutoTimer.   If there are cases where you don't want to record telemetry when returning from an error (seems to be the case in nsCacheService::SearchCacheDevices) then at least replace 

  Telemetry::Accumulate(Telemetry::CACHE_DEVICE_SEARCH, static_cast<PRInt32>(PR_Now() - start));

  with AccumulateTimeDelta(Telemetry::CACHE_DEVICE_SEARCH, start);

I wonder if the memory cache is going to show useful metrics--I'd expect it to always return in <1 ms.   No harm in trying...
Comment 3 User image OFFLINE UNTIL 27 FEB 2017 Nicholas Hurley [:nwgh][:hurley] (also hurley@todesschaf.org) 2011-10-27 16:57:15 PDT
https://tbpl.mozilla.org/?tree=Try&rev=975d9ec99a99
Comment 4 User image OFFLINE UNTIL 27 FEB 2017 Nicholas Hurley [:nwgh][:hurley] (also hurley@todesschaf.org) 2011-10-28 11:28:47 PDT
Created attachment 570316 [details] [diff] [review]
updated patch

Updated to use autotimers everywhere. This is the patch that was pushed to try above.
Comment 5 User image OFFLINE UNTIL 27 FEB 2017 Nicholas Hurley [:nwgh][:hurley] (also hurley@todesschaf.org) 2011-10-28 11:29:11 PDT
Try looks good
Comment 7 User image Matt Brubeck (:mbrubeck) 2011-10-31 11:21:56 PDT
https://hg.mozilla.org/mozilla-central/rev/42be7e75a8fe

Note You need to log in before you can comment on or make changes to this bug.