429336 - Upgrade to sqlite 3.5.8

Status: RESOLVED WONTFIX

(Toolkit :: Storage, defect)

Description

[Shawn Wilsher :sdwilsh]

patching coming soonish.  conversation with schrep indicated that we should take this...

Comment 1

[Shawn Wilsher :sdwilsh]

Attachment: v1.0

sqlite changes omitted - file is way to big to attach with them

Comment 2

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

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

r=shaver, godspeed

Comment 3

[Shawn Wilsher :sdwilsh]

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

also seeking approval to land the sqlite changes, which are too big to attach.

Comment 4

[Mike Beltzner [:beltzner, not reading bugmail]]

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

a1.9=beltzner

Comment 5

[Shawn Wilsher :sdwilsh]

Checking in configure.in;
new revision: 1.1982; previous revision: 1.1981
Checking in db/sqlite3/README.MOZILLA;
new revision: 1.26; previous revision: 1.25
Checking in db/sqlite3/src/sqlite3.c;
new revision: 1.17; previous revision: 1.16
Checking in db/sqlite3/src/sqlite3.h;
new revision: 1.20; previous revision: 1.19

and to stop us from constantly changing the $Id$ in the files...
11 mozilla $ cvs admin -ko db/sqlite3/src/sqlite3.*
RCS file: /cvsroot/mozilla/db/sqlite3/src/sqlite3.c,v
done
RCS file: /cvsroot/mozilla/db/sqlite3/src/sqlite3.h,v
done

Comment 6

[Shawn Wilsher :sdwilsh]

and a bustage fix because the def file contained something we don't have anymore...

Checking in src/sqlite.def;
new revision: 1.10; previous revision: 1.9

Comment 7

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

Note: per discussion in the SQLite users mailing list <http://www.mail-archive.com/sqlite-users%40sqlite.org/msg33263.html>, 3.5.8 contains a performance regression for left outer joins from the fix for SQLite ticket #3015 <http://www.sqlite.org/cvstrac/tktview?tn=3015>.

We use some of those (almost exclusively in places) <http://mxr.mozilla.org/mozilla/search?string=left%20outer%20join>, so we should be on the lookout for effects on our performance and track the work to fix the regression closely.

Comment 8

[Daniel Holbert [:dholbert]]

Backed this out on behalf of sdwilsh, to see if it caused the mac & linux tinderbox oranges. (with "reftest 0/0/0")

Checking in configure.in;
/cvsroot/mozilla/configure.in,v  <--  configure.in
new revision: 1.1983; previous revision: 1.1982
done
Checking in db/sqlite3/README.MOZILLA;
/cvsroot/mozilla/db/sqlite3/README.MOZILLA,v  <--  README.MOZILLA
new revision: 1.27; previous revision: 1.26
done
Checking in db/sqlite3/src/sqlite.def;
/cvsroot/mozilla/db/sqlite3/src/sqlite.def,v  <--  sqlite.def
new revision: 1.11; previous revision: 1.10
done
Checking in db/sqlite3/src/sqlite3.c;
/cvsroot/mozilla/db/sqlite3/src/sqlite3.c,v  <--  sqlite3.c
new revision: 1.18; previous revision: 1.17
done
Checking in db/sqlite3/src/sqlite3.h;
/cvsroot/mozilla/db/sqlite3/src/sqlite3.h,v  <--  sqlite3.h
new revision: 1.21; previous revision: 1.20
done

Comment 9

[Shawn Wilsher :sdwilsh]

I do not have time to deal with this.

Comment 10

[Jesse Ruderman]

During the brief period this was in the tree, Firefox took a few extra seconds to start up, and froze on shutdown.

Comment 11

[Daniel Holbert [:dholbert]]

Mac reftests went back to normal after this was backed out.  (Linux is still running)

Broken cycle (pre-backout):
  http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1208458696.1208463548.18519.gz
Working cycle (post-backout):
  http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1208466480.1208469284.32094.gz

Comment 12

[Daniel Holbert [:dholbert]]

Linux went back to normal as well.

Comment 13

[Jim Jeffery not reading bug-mail 1/2/11]

I'm not sure, but I think the update also was causing hangs when opening the Library, either bookmarks or history.  History was really a long delay, Vista went non-responding on several occasions, and with the Library open, opening any other tabs, or refreshing them produced non-repsonding.

My Places.sqlite is 4.8 meg, and takes normally about 2 secs to load, it was taking upwards of 40+ secs, most of the time spent 'Not Repsonding'. 

I have gone back to build:
Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9pre) Gecko/2008041702 Minefield/3.0pre Firefox/3.0 ID:2008041702 
which was stable, now I'm crashing on restore after being minimized for an unknown period of time, and this is in the Event Viewer:

Faulting application firefox.exe, version 1.9.0.3027, time stamp 0x48072c3c, faulting module sqlite3.dll, version 3.5.4.1, time stamp 0x48072c99, exception code 0xc0000005, fault offset 0x0000d872, process id 0xd00, application start time 0x01c8a0cd622c93e0. 

Thus my fears that the update may have corrupted something.

Comment 14

[Jim Jeffery not reading bug-mail 1/2/11]

Confirmed, the backout of this update no longer is causing the 'Not repsonding' and my History is back to 'Normal' open time, about 2 secs on this old tired machine. 

Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9pre) Gecko/2008041717 Minefield/3.0pre Firefox/3.0 ID:2008041717 <-Latest hourly - the SQLite update.

Comment 15

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

I compiled and ran reftests both with and without 3.5.8.  As far as I can tell, the reftests ran without incident both times, although I didn't get output from them in either case, so it's hard to know for sure.

Comment 16

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

(In reply to comment #15)
> I compiled and ran reftests both with and without 3.5.8.  As far as I can tell,
> the reftests ran without incident both times, although I didn't get output from
> them in either case, so it's hard to know for sure.

Erm, but that's because I was running them in an opt build.  I reran both in debug builds, which did output results, and both builds passed all reftests.

So I'm not sure what the issue is.  Maybe the left outer join performance regression that probably caused the places slowness on startup caused the test infrastructure to time out?  I don't have a better idea.

Comment 17

[Mike Schroepfer]

Given comment 7 and later and the phase of the release should we punt an upgrade until a future version?  Any driving reason we need 3.5.8 (+ fix for 3015) now?

Comment 18

[Damon Sicore (:damons)]

This seems really really scary.  Given comments 6-14, I'd vote to not take this at this point.

Re-nom if you disagree.

Comment 19

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

(In reply to comment #17)
> Given comment 7 and later and the phase of the release should we punt an
> upgrade until a future version?  Any driving reason we need 3.5.8 (+ fix for
> 3015) now?

3.5.8 fixes "a bug in the SQLITE_SECURE_DELETE option that was causing Firefox crashes."  That bug is what caused 3.5.7 to be particularly crashy for us, but the bug is present in 3.5.4.2 and could cause crashes there too.

However, if we aren't experiencing a statistically noticeable number of such crashes, then this could wait for a future release, as the other fixes we'd pick up are either not particularly important/relevant or only affect extensions.


(In reply to comment #18)
> This seems really really scary.  Given comments 6-14, I'd vote to not take this
> at this point.

That seems reasonable to me, given where we are in the cycle.  Let's circle around to this on a future point release.

Comment 20

[Shawn Wilsher :sdwilsh]

I agree.