445042 - Upgrade to SQLite 3.6.0

Status: RESOLVED WONTFIX

(Core :: SQLite and Embedded Database Bindings, defect, P1)

Description

[Myk Melez [:myk] [@mykmelez]]

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.

Comment 1

[Amarjeet Singh Rai]

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.

Comment 2

[Shawn Wilsher :sdwilsh]

When it's out of beta, I'll be happy to generate the patch for this.  Until then, this bug will be on hold.

Comment 3

[Myk Melez [:myk] [@mykmelez]]

(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?

Comment 4

[Shawn Wilsher :sdwilsh]

(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.

Comment 5

[Myk Melez [:myk] [@mykmelez]]

(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.

Comment 6

[Shawn Wilsher :sdwilsh]

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.

Comment 7

[Peter Weilbacher]

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.)

Comment 8

[Peter Weilbacher]

(In reply to comment #7)
> at least that's how it was communicated on the

sqlite-dev and sqlite-users lists.

Comment 9

[Shawn Wilsher :sdwilsh]

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

Comment 10

[Shawn Wilsher :sdwilsh]

Attachment: v1.0

The try server isn't working on windows - the one platform I'm worried about.  Not because of this change though.

Comment 11

[Shawn Wilsher :sdwilsh]

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

shaver is a bit busy, so we'll try myk instead.

Comment 12

[(no longer active)]

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?

Comment 13

[Amarjeet Singh Rai]

I hope this fixes the performance issues I'm having with loading large bookmarks on Fx3.

Comment 14

[Shawn Wilsher :sdwilsh]

(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 15

[Myk Melez [:myk] [@mykmelez]]

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

Comment 16

[Shawn Wilsher :sdwilsh]

Pushed to mozilla-central:
http://hg.mozilla.org/mozilla-central/index.cgi/rev/fe247eb913e2
http://hg.mozilla.org/mozilla-central/index.cgi/rev/1bef2be14d0f

Comment 17

[Dave Townsend [:mossop]]

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

Comment 18

[Sergey Yanovich]

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?

Comment 19

[Dave Townsend [:mossop]]

(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.

Comment 20

[Shawn Wilsher :sdwilsh]

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.

Comment 21

[Henrik Skupin [:whimboo][⌚️UTC+2]]

(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?

Comment 22

[Dave Townsend [:mossop]]

(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

Comment 23

[Peter Weilbacher]

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.)

Comment 24

[Shawn Wilsher :sdwilsh]

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.