Closed
Bug 1272855
Opened 9 years ago
Closed 8 years ago
dom/indexedDB/test/unit/test_quotaExceeded_recovery.js incurs ~3.5 GB of I/O
Categories
(Core :: Storage: IndexedDB, defect)
Core
Storage: IndexedDB
Tracking
()
RESOLVED
FIXED
mozilla52
Tracking | Status | |
---|---|---|
firefox52 | --- | fixed |
People
(Reporter: gps, Assigned: janv)
References
(Blocks 1 open bug)
Details
(Whiteboard: btpp-followup-2016-05-31)
Attachments
(1 file)
1.52 KB,
patch
|
baku
:
review+
|
Details | Diff | Splinter Review |
dom/indexedDB/test/unit/test_quotaExceeded_recovery.js incurs ~3.5 GB of I/O at the OS level when interacting with a SQLite database. ~3 GB of this is interacting with the WAL journal.
Not all of this I/O hits disk. But this test easily sticks out as one of the most significant consumers of I/O during xpcshell tests.
If the test could be refactored to use less I/O, it would help tests execute faster.
Comment 1•9 years ago
|
||
Andrew may be interested in investigating this :)
Flags: needinfo?(bugmail)
Whiteboard: btpp-followup-2016-05-31
Comment 2•9 years ago
|
||
Oh, sorry, Jan probably knows what's up here.
Flags: needinfo?(bugmail) → needinfo?(jvarga)
Assignee | ||
Comment 3•9 years ago
|
||
Testing a patch on try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=6fdd640bf310
I lowered the database file by a factor of 8 on Android and 4 on other platforms.
Flags: needinfo?(jvarga)
Assignee | ||
Comment 4•9 years ago
|
||
android test_quotaExceeded_recovery.js went down to 62918ms which is comparable to test_objectStore_openKeyCursor.js which took 45368ms
Comment 5•9 years ago
|
||
gps, do you have a # in mind of what you'd like to see for the time take and/or I/O used?
Flags: needinfo?(gps)
Reporter | ||
Comment 6•9 years ago
|
||
Wall time in automation is difficult because values can be all over the map depending on hardware. But this test is likely slow in automation because of excessive I/O.
I don't really have a specific target: I filed this bug because this test was the worst I/O abuser in the xpcshell harness. The reduction of 8x and 4x in the existing patch is definitely an improvement and I'm happy with it. If you can do more, I'd say "why not?" But at some point we'd probably sacrifice testing robustness, right? Not sure where you can draw the line in this case since I don't know the details of what's being tested.
The big perf/I/O kill in SQLite is transaction commit count and their size. If you can minimize those (mostly commit count), that should have the biggest impact on perf.
FWIW I added a preference for Places in bug 1272025 to change SQLite settings to use an in-memory transaction and reduce flushing durability. This makes transactions really fast at the cost of durability. I'm not sure if IndexedDB could be placed in a similar low-durability mode for certain tests. My contention in that bug and the bug(s) it blocks is that changing the SQLIte settings shouldn't change its behavior. Durability should only come into play for crashes, power loss, etc. So I argue that the default high durability mode doesn't really provide us much in test value except the possibility of discovering race conditions. Something to think about it. Of course, if you are testing quotas, you may care about the on-disk size of the WAL, so in-memory transactions may undermine the test...
Flags: needinfo?(gps)
Assignee | ||
Comment 7•8 years ago
|
||
Assignee | ||
Comment 8•8 years ago
|
||
Updated•8 years ago
|
Attachment #8802593 -
Flags: review?(amarchesini) → review+
Pushed by jvarga@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/525cf19ff647
dom/indexedDB/test/unit/test_quotaExceeded_recovery.js incurs ~3.5 GB of I/O; r=baku
Comment 10•8 years ago
|
||
bugherder |
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
status-firefox52:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla52
You need to log in
before you can comment on or make changes to this bug.
Description
•