Closed Bug 673727 Opened 10 years ago Closed 10 years ago

Report sqlite waiting on/off main thread via telemetry

Categories

(Toolkit :: Storage, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla8

People

(Reporter: taras.mozilla, Assigned: taras.mozilla)

References

Details

(Whiteboard: [inbound])

Attachments

(1 file)

No description provided.
Attached patch measurementsSplinter Review
Attachment #548324 - Flags: review?(mak77)
Assignee: nobody → tglek
Status: NEW → ASSIGNED
OS: Linux → All
Hardware: x86 → All
Comment on attachment 548324 [details] [diff] [review]
measurements

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

::: storage/src/TelemetryVFS.cpp
@@ +75,4 @@
>     Telemetry::MOZ_SQLITE_OTHER_SYNC }
>  };
>  
> +class AutoIOWaitThreadTimer {

add a javadoc above the class with a really brief explanation of what this is for.

nit: I find IOThreadAutoTimer clearer, but I'm not english mother tongue so I trust your language skills, what do you think?

@@ +89,5 @@
> +               , (TimeStamp::Now() - start).ToMilliseconds());
> +  }
> +
> +private:
> +const TimeStamp start;

wrong indentation

@@ +122,4 @@
>  int
>  xRead(sqlite3_file *pFile, void *zBuf, int iAmt, sqlite_int64 iOfst)
>  {
> +  AutoIOWaitThreadTimer aiwtt;

nit: I'd just call these "autoTimer"

::: toolkit/components/telemetry/TelemetryHistograms.h
@@ +121,5 @@
>  HISTOGRAM(MOZ_SQLITE_OTHER_SYNC, 1, 10000, 10, EXPONENTIAL, "Time spent on sqlite fsync() (ms)")
>  HISTOGRAM(STARTUP_MEASUREMENT_ERRORS, 1, 3, 4, LINEAR, "Flags errors in startup calculation()")
>  HISTOGRAM(NETWORK_DISK_CACHE_OPEN, 1, 10000, 10, EXPONENTIAL, "Time spent opening disk cache (ms)")
> +HISTOGRAM(MOZ_SQLITE_MAIN_THREAD_WAIT, 1, 3000, 10, EXPONENTIAL, "Time spent waiting on sqlite io on main thread (ms)")
> +HISTOGRAM(MOZ_SQLITE_OTHER_THREAD_WAIT, 1, 3000, 10, EXPONENTIAL, "Time spent waiting on sqlite io off main thread (ms)")

correct case SQLite IO

s/OTHER_THREAD/ASYNC_THREAD/ since we more often talk about sync and async apis in documentation

nit: I'd add _MS suffix to these probes names, so it's cleared in code what they expect.
Attachment #548324 - Flags: review?(mak77) → review+
sorry, I changed my mind, OTHER_THREAD is fine, so don't change it
http://hg.mozilla.org/integration/mozilla-inbound/rev/2c58da20a79a
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Sorry, old habits die hard
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Whiteboard: [inbound]
http://hg.mozilla.org/mozilla-central/rev/2c58da20a79a
Status: REOPENED → RESOLVED
Closed: 10 years ago10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla8
You need to log in before you can comment on or make changes to this bug.