Closed Bug 723124 Opened 8 years ago Closed 8 years ago

Telemetry for time needed for idle frecency update

Categories

(Toolkit :: Places, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla13

People

(Reporter: mak, Assigned: mak)

References

(Blocks 1 open bug)

Details

Attachments

(1 file, 1 obsolete file)

On idle all frecencies are updated, this is expensive, we should measure it.
Attached patch patch v1.0 (obsolete) — Splinter Review
Paolo figured out a really simple way to replace the histograms check that now we can use, so I stole it :)
Attachment #593823 - Flags: review?(dietrich)
Comment on attachment 593823 [details] [diff] [review]
patch v1.0

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

r=me

::: toolkit/components/places/nsNavHistory.cpp
@@ +309,5 @@
>  }
>  
> +// Used to telemetry execution time of an async query.
> +class AsyncStatementTelemetryTimer : public AsyncStatementCallback
> +{

This seems pretty generic, other than the telemetry histogram identifier. Would it be worth putting this in it's own file or with storage helpers, and letting it take the identifier as an argument as well?
Attachment #593823 - Flags: review?(dietrich) → review+
good idea on the identifier.
In many cases we'll have some callback doing something else, this is just usable on statements that don't have a callback already.
I may move it to places/Helpers.h to make it more available, it's not something for Storage due to the above.
Attached patch patch v1.1Splinter Review
Addressed comments
Attachment #593823 - Attachment is obsolete: true
https://hg.mozilla.org/mozilla-central/rev/a447bac56f0a
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.