Closed Bug 445042 Opened 12 years ago Closed 12 years ago

Upgrade to SQLite 3.6.0

Categories

(Toolkit :: Storage, defect, P1)

defect

Tracking

()

RESOLVED WONTFIX
mozilla1.9.1a2

People

(Reporter: myk, Assigned: sdwilsh)

Details

Attachments

(1 file)

SQLite version 3.6.0 will be released soon.  It contains all the fixes from 3.5 releases since we branched off at 3.5.4 (including some we wanted to get for Fx3) along with a variety of other improvements.  And it's going to be the stable SQLite branch once released.  So we should upgrade to it on the development branch for the next Firefox release.
Flags: blocking1.9.1?
Version: unspecified → Trunk
Blocks: 417037
Blocks: 442668
Blocks: 445525
Blocks: 443577
Blocks: 444446
Assignee: nobody → sdwilsh
Target Milestone: --- → mozilla1.9.1a1
2008-July-16 - Version 3.6.0 beta

Version 3.6.0 makes changes to the VFS object in order to make SQLite more easily portable to a wider variety of platforms. There are potential incompatibilities with some legacy applications. See the 35to36.html document for details.

Many new interfaces are introduced in version 3.6.0. The code is very well tested and is appropriate for use in stable systems. We have attached the "beta" designation only so that we can make tweaks to the new interfaces in the next release without having to declare an incompatibility.

========================================================

We should implement this into branch for 3.1 now. Or at least the trunk.
When it's out of beta, I'll be happy to generate the patch for this.  Until then, this bug will be on hold.
(In reply to comment #2)
> When it's out of beta, I'll be happy to generate the patch for this.  Until
> then, this bug will be on hold.

If we want to take it, then I'd think getting it in early so we can raise our concerns during the beta development process would be preferable, no?
(In reply to comment #3)
> If we want to take it, then I'd think getting it in early so we can raise our
> concerns during the beta development process would be preferable, no?
I've been using it with no issue locally.  I'm pretty (based on past upgrade discussions) sure drivers would not like me landing a beta sqlite in the tree.
(In reply to comment #4)
> (In reply to comment #3)
> > If we want to take it, then I'd think getting it in early so we can raise our
> > concerns during the beta development process would be preferable, no?
> I've been using it with no issue locally.  I'm pretty (based on past upgrade
> discussions) sure drivers would not like me landing a beta sqlite in the tree.

I've been part of a couple upgrade attempts, perhaps not all of them, which involved stable releases that caused regressions.  Those could indeed have made drivers wary of SQLite upgrades.  It certainly made me wary to the extent that I recommended not upgrading to a new SQLite release at one point, despite wanting the fixes in the new release.

But those were all late in the development cycle, whereas we're currently in pre-alpha.

That doesn't mean we should land something with significant regressions, but if it doesn't appear have any significant regressions, as far as we can tell via automated and manual testing, then I wouldn't think its beta status should keep us from landing it at this stage in the cycle, if we know we want it for final.
It's my understanding that the actual release will be out this week.  I usually test the releases myself when it's in beta to make sure our test harness doesn't find any issues.
As far as I understand, 3.6.0 _is_ the release, at least that's how it was communicated on the  (drh just signifies new .0 releases as alpha or beta because a design flaw might have crept into the changes. But as I didn't see any big outcry on the sqlite-dev list, I guess that's unlikely.)

Although I saw last night that I busted the OS/2 part of SQLite 3.6.0 by forgetting to remove debug output. (Otherwise I would also vote to upgrade early to see if this release really fixes the startup time problem and the dependent bugs.)
(In reply to comment #7)
> at least that's how it was communicated on the

sqlite-dev and sqlite-users lists.
Blocks: 446118
Just the sqlite file changes.  Running through the try server to make sure my changes to our code work out.
http://hg.mozilla.org/users/sdwilsh_shawnwilsher.com/storage-async/index.cgi/file/82e023c99505/sqlite.3.6.0
Status: NEW → ASSIGNED
Attached patch v1.0Splinter Review
The try server isn't working on windows - the one platform I'm worried about.  Not because of this change though.
Attachment #331379 - Flags: review?(shaver)
Whiteboard: [has patch][needs review shaver]
Priority: -- → P1
Target Milestone: mozilla1.9.1a1 → mozilla1.9.1a2
Comment on attachment 331379 [details] [diff] [review]
v1.0

shaver is a bit busy, so we'll try myk instead.
Attachment #331379 - Flags: review?(shaver) → review?(myk)
Whiteboard: [has patch][needs review shaver] → [has patch][needs review myk]
Blocks: 448114
Shawn, can you incorporate the patch from bug 446208 into this patch as well, so that the 3.6.0 version we would end up using would include the fix for that bug as well?
Depends on: 446208
I hope this fixes the performance issues I'm having with loading large bookmarks on Fx3.
No longer depends on: 446208
(In reply to comment #12)
> Shawn, can you incorporate the patch from bug 446208 into this patch as well,
> so that the 3.6.0 version we would end up using would include the fix for that
> bug as well?
commented in that bug

(In reply to comment #13)
> I hope this fixes the performance issues I'm having with loading large
> bookmarks on Fx3.
You should be filing bugs in places then.  Chances are, this won't help you (but we are happy to help look into other perf issues; just file a new bug please)

Comment on attachment 331379 [details] [diff] [review]
v1.0

Looks good, builds properly, tests all pass, and Firefox with this patch (and the new SQLite code) has been running without apparent incident on my laptop all day. r=myk
Attachment #331379 - Flags: review?(myk) → review+
Whiteboard: [has patch][needs review myk] → [has patch][has review][can land]
Pushed to mozilla-central:
http://hg.mozilla.org/mozilla-central/index.cgi/rev/fe247eb913e2
http://hg.mozilla.org/mozilla-central/index.cgi/rev/1bef2be14d0f
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Flags: in-testsuite-
Flags: in-litmus-
Resolution: --- → FIXED
Whiteboard: [has patch][has review][can land]
I've backed this out due to the performance regressions on Linux and Windows.

http://graphs.mozilla.org/#show=911692,911653,912146&sel=1217755086,1217951807
http://hg.mozilla.org/mozilla-central/index.cgi/rev/9a9be5689aeb
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
I thought to ask this question when 3.5.9 was backed out.

sqlite3 at 3.5.4 had some serious deficiencies. F.e., it incorrectly handles LEFT JOINs if ON clause contains a subset of primary key in the right table (sqlite ticket 3015). Fixing those bugs requires introducing new checks into sqlite engine, which leads to correct functioning at the cost of performance.

The question is:
is it OK to decline critical SQL fixes like ticket 3015 on performance reasons? And what is the plan, when security holes are discovered in sqlite that put the browser under risk of exploit?
(In reply to comment #18)
> The question is:
> is it OK to decline critical SQL fixes like ticket 3015 on performance reasons?
> And what is the plan, when security holes are discovered in sqlite that put the
> browser under risk of exploit?

It becomes a risk-reward toss up. It has been backed out not because it definitely won't be taken, but because (as far as I can see) there was no anticipated and accepted performance degradation. If the regression is deemed to be acceptable by drivers then it can go back in again.
I'll note that Firefox 3.0.1+ (and 1.9.0.1+) is using SQLite 3.5.9.  It was backed out of mozilla-central though because the performance regression was more defined.

I need to get my hands on a hardware windows box to take a look at why we are so much slower on startup with the new sqlite.
(In reply to comment #20)
> I need to get my hands on a hardware windows box to take a look at why we are
> so much slower on startup with the new sqlite.

Does that only happen on Windows or is Firefox on OS X also affected?
(In reply to comment #21)
> (In reply to comment #20)
> > I need to get my hands on a hardware windows box to take a look at why we are
> > so much slower on startup with the new sqlite.
> 
> Does that only happen on Windows or is Firefox on OS X also affected?

The graphs show about a 12% Ts regression on Windows, 48% on Linux, no noticable change on OSX
If you want to try again maybe you should go directly for SQLite 3.6.1 that was just released. Although I couldn't test that on my setup yet, it contains many performance enhancements... (And removes the printfs that I had forgotten earlier in the OS/2 backend.)
We can try 3.6.1 (bug 449443), but I suspect those perf issues will still be around.  I am still waiting on my hardware windows box to get purify going on it and see what's up.
No longer blocks: 417037, 442668, 443577, 444446, 445525, 446118, 448114
Status: REOPENED → RESOLVED
Closed: 12 years ago12 years ago
Flags: blocking1.9.1?
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.