Closed
Bug 673727
Opened 13 years ago
Closed 13 years ago
Report sqlite waiting on/off main thread via telemetry
Categories
(Toolkit :: Storage, defect)
Toolkit
Storage
Tracking
()
RESOLVED
FIXED
mozilla8
People
(Reporter: taras.mozilla, Assigned: taras.mozilla)
References
Details
(Whiteboard: [inbound])
Attachments
(1 file)
3.40 KB,
patch
|
mak
:
review+
|
Details | Diff | Splinter Review |
No description provided.
Assignee | ||
Comment 1•13 years ago
|
||
Attachment #548324 -
Flags: review?(mak77)
Updated•13 years ago
|
Assignee: nobody → tglek
Status: NEW → ASSIGNED
OS: Linux → All
Hardware: x86 → All
Comment 2•13 years ago
|
||
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+
Comment 3•13 years ago
|
||
sorry, I changed my mind, OTHER_THREAD is fine, so don't change it
Assignee | ||
Comment 4•13 years ago
|
||
http://hg.mozilla.org/integration/mozilla-inbound/rev/2c58da20a79a
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Assignee | ||
Comment 5•13 years ago
|
||
Sorry, old habits die hard
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Whiteboard: [inbound]
Comment 6•13 years ago
|
||
http://hg.mozilla.org/mozilla-central/rev/2c58da20a79a
Status: REOPENED → RESOLVED
Closed: 13 years ago → 13 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla8
You need to log in
before you can comment on or make changes to this bug.
Description
•