Closed Bug 723126 Opened 9 years ago Closed 9 years ago

Telemetry for time needed for idle maintenance

Categories

(Toolkit :: Places, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla13

People

(Reporter: mak, Assigned: mak)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

We don't have a good idea of how much maintenance may take in the wild. It happens on idle but may still be an heavy task disrupting video streaming and such.
Attached patch patch v1.0Splinter Review
Attachment #593863 - Flags: review?(dietrich)
Comment on attachment 593863 [details] [diff] [review]
patch v1.0

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

::: toolkit/components/places/PlacesDBUtils.jsm
@@ +90,5 @@
>      else {
> +      // All tasks have been completed.
> +      // Telemetry the time it took for maintenance, if a start time exists.
> +      if (aTasks.telemetryStart) {
> +        try {

remove the try/catch, unnecessary.

@@ +92,5 @@
> +      // Telemetry the time it took for maintenance, if a start time exists.
> +      if (aTasks.telemetryStart) {
> +        try {
> +          Services.telemetry.getHistogramById("PLACES_IDLE_MAINTENANCE_TIME_MS")
> +                            .add(Date.now() - aTasks.telemetryStart);

should clear the telemetryStart value.

@@ +138,5 @@
>        this.checkIntegrity
>      , this.checkCoherence
>      , this._refreshUI
>      ]);
> +    tasks.telemetryStart = Date.now();

please add underscore since it's fake-private. here and elsewhere.
Attachment #593863 - Flags: review?(dietrich) → review+
with fixed comments

https://hg.mozilla.org/integration/mozilla-inbound/rev/a31efbfbba7b
Target Milestone: --- → mozilla13
https://hg.mozilla.org/mozilla-central/rev/a31efbfbba7b
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Depends on: 724805
You need to log in before you can comment on or make changes to this bug.