Closed
Bug 1310361
(SQLite3.15.0)
Opened 8 years ago
Closed 8 years ago
Upgrade to SQLite 3.15.0
Categories
(Toolkit :: Storage, defect, P3)
Toolkit
Storage
Tracking
()
RESOLVED
FIXED
mozilla52
Tracking | Status | |
---|---|---|
firefox52 | --- | fixed |
People
(Reporter: ionnv, Assigned: RyanVM)
References
()
Details
Attachments
(1 file)
(deleted),
patch
|
mak
:
review+
|
Details | Diff | Splinter Review |
https://sqlite.org/news.html
https://sqlite.org/releaselog/3_15_0.html
SQLite version 3.15.0 is a regularly scheduled maintenance release. The key feature in this release is the added support for row values. There are also other enhancements and fixes for a number of obscure bugs.
The 3.15.0 release uses about 7% fewer CPU cycles than 3.14.2. Most of the improvement in this release is in the SQL parser, query planner, and byte-code generator (the front-end) corresponding to the sqlite3_prepare_v2() interface. Overall, version 3.15.0 uses about half as much CPU time as version 3.8.1 (2013-10-17). These performance measurements are made using the "speedtest1.c" workload on x64 compiled with gcc and -Os. Performance improvements may vary with different platforms and workloads.
Depends on: SQLite3.14.1
Assignee | ||
Comment 1•8 years ago
|
||
Assignee: nobody → ryanvm
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Updated•8 years ago
|
Priority: -- → P3
Assignee | ||
Comment 2•8 years ago
|
||
Try looks fine, but not feeling any terrible sense of urgency in getting this reviewed and landed either. Let's give it a week or so and be on the lookout for any point releases before we commit to taking it.
Assignee | ||
Comment 3•8 years ago
|
||
Talos results are mostly uninteresting, but there does appear to be a pretty catastrophic hit to startup file IO.
https://treeherder.mozilla.org/perf.html#/compare?originalProject=mozilla-central&originalRevision=de5d73a0568d&newProject=try&newRevision=b47b69dabfe198b93fb2270cae845dfe9373aa6a&framework=1&filter=nonmain_startup_fileio&showOnlyImportant=1
Comment 4•8 years ago
|
||
that is huge, it is not on the main thread, but it would be really interesting to know why we are going from ~40K bytes -> ~400K bytes for the non main thread during startup. Is it possible that we are doing something that can be fixed with a config change or other options. Maybe some caching mode or other fileIO internal to SQLite? It could be that this depends on other libraries so we are loading more dll's.
Comment 5•8 years ago
|
||
The changelog doesn't raise any possible culprits, may be some internal changes. let's see if Richard has any insights about this.
Note it's also possible Talos is now measuring something that was happening just after startup and now happens earlier. But we clearly need to clarify this before we can upgrade.
Flags: needinfo?(drh)
Comment 6•8 years ago
|
||
(In reply to Marco Bonardo [::mak] from comment #5)
> let's see if Richard has any insights about this.
No ideas. But we will puzzle over it today and see what we can come up with. Nothing like this appears in our own instrumentation.
Am I correct in understanding that the problem only comes up on Windows7 with 32-bit compiles?
Comment 7•8 years ago
|
||
afaict yes, from what I see Ryan triggered a full run on all OSes and this is the only result that came out with a large delta.
Joel, do we have aby other talos measurement that could tell us of IO remained the same in a full run, rather than only on startup? While it would still be bad to regress startup, at least we'd know it's a shifting and not new IO.
Flags: needinfo?(jmaher)
Comment 8•8 years ago
|
||
we only do this measurement on windows 7, xperf allows us to easily measure this.
I don't have overall measurements, only startup based measurements- that is a good point that we could be seeing data from post startup taking place during startup. I don't have a lot of data from the try push, but I do not see any large changes in startup tests (ts_paint, sessionrestore)
Flags: needinfo?(jmaher)
Assignee | ||
Comment 9•8 years ago
|
||
(In reply to D. Richard Hipp from comment #6)
> No ideas. But we will puzzle over it today and see what we can come up
> with. Nothing like this appears in our own instrumentation.
I'm happy to run Try pushes of various intermediate revisions if you want to try bisecting it on our end. Let me know :)
Comment hidden (obsolete) |
Comment hidden (obsolete) |
Comment 12•8 years ago
|
||
(In reply to Ryan VanderMeulen [:RyanVM] from comment #9)
>
> I'm happy to run Try pushes of various intermediate revisions if you want to
> try bisecting it on our end. Let me know :)
There are 173 check-ins in between version 3.14.2 and 3.15.0, so about 8 Trys to isolate the problem. The first bisect can be found in https://www.sqlite.org/tmp/bug-1310361-bisect-1.zip - if you can run that on the Try server that would be great. Let me know the result and I can give you the next one.
Should we continue the bisect off-line? You can contact me directly at drh@sqlite.org or by phone at 704.948.4565 if that helps.
Assignee | ||
Comment 13•8 years ago
|
||
Richard and I chatted more via email. I ran a series of Try pushes and convinced at this point that whatever we're seeing is a phantom regression. I pushed an empty commit to Try on top of mozilla-central tip and I see the exact same regression relative to m-c as I saw with the SQLite 3.15.0 update applied (the difference is negligible). So for some reason, we're doing something awful on trunk vs. Try that's causing massive perceived regressions in that test, but those don't translate to an actual problem AFAICT.
Flags: needinfo?(drh)
Comment 14•8 years ago
|
||
At this point, I'd suggest from now on to push current tip to Try and the Sqlite update on top of it and do a Try to Try comparison. Could we start doing that now to be 100% sure we have proper data?
Flags: needinfo?(ryanvm)
Comment 15•8 years ago
|
||
sorry I didn't see this earlier, but it could be related to bug 1274018.
Assignee | ||
Comment 16•8 years ago
|
||
(In reply to Marco Bonardo [::mak] from comment #14)
> At this point, I'd suggest from now on to push current tip to Try and the
> Sqlite update on top of it and do a Try to Try comparison. Could we start
> doing that now to be 100% sure we have proper data?
That's exactly what I did in comment 13. The difference was negligible.
Flags: needinfo?(ryanvm)
Assignee | ||
Comment 17•8 years ago
|
||
FYI, I'm waiting on the outcome of bug 1310152 before pushing forward with landing here in case we end up needing a fix in SQLite.
Comment 18•8 years ago
|
||
(In reply to Ryan VanderMeulen [:RyanVM] from comment #17)
> FYI, I'm waiting on the outcome of bug 1310152 before pushing forward with
> landing here in case we end up needing a fix in SQLite.
I don't think we should block on that, from the first analysis it sounds unlikely to be a problem in SQLite, it's more likely we are doing something wrong with its mutex. Regardless, we are not making the situation worse by upgrading.
Updated•8 years ago
|
Attachment #8801486 -
Flags: review+
Comment 19•8 years ago
|
||
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/b42a7d4ec7a2
Upgrade to SQLite 3.15.0. r=mak
Assignee | ||
Comment 20•8 years ago
|
||
bugherder |
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
status-firefox52:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla52
Assignee | ||
Updated•8 years ago
|
Blocks: SQLite3.15.1
You need to log in
before you can comment on or make changes to this bug.
Description
•