I've been thinking that we should try to use Snappy (https://code.google.com/p/snappy) for a while and so I finally ran some numbers. Our test case was basically adding 5000 entries into an object store. Each of the entries has a data member with either 32k or 1k worth of string data. Times for adding the data and the final file sizes are: ========== No Snappy: ========== Storing 32k fragments: Put 5001 ids in 3617 ms Put 5001 ids in 3728 ms Put 5001 ids in 3724 ms Put 5001 ids in 3662 ms Final size: 481728k Storing 1k fragments: Put 5001 ids in 631 ms Put 5001 ids in 615 ms Put 5001 ids in 668 ms Put 5001 ids in 666 ms Final size: 17600k ============= Using Snappy: ============= Storing 32k fragments: Put 5001 ids in 4416 ms Put 5001 ids in 4538 ms Put 5001 ids in 4437 ms Put 5001 ids in 4464 ms Final size: 54912k Storing 1k fragments: Put 5001 ids in 776 ms Put 5001 ids in 804 ms Put 5001 ids in 795 ms Put 5001 ids in 796 ms Put 5001 ids in 772 ms Final size: 4192k That's an order of magnitude difference in file size for the large data case! The slowdown is unfortunate, but I think it's probably worth it. We'll need to think through data migration from old databases a little, but that shouldn't be too hard. Posting the patch here now so that I don't lose it. The patch also doesn't include the snappy source files just to keep the size down.

This patch is the whole thing (minus the snappy source). Everything seems to work well! Migrates old databases to the new compression format, changes how we do data versioning too.

Matt, heads up, I've gone ahead and integrated Snappy here.

Comment on attachment 575803 [details] [diff] [review] Patch, v1 [not for checkin, missing snappy source] Review of attachment 575803 [details] [diff] [review]: ----------------------------------------------------------------- r=me on the non-build-goop stuff. Though this version stuff is way too complex :( ::: dom/indexedDB/IDBObjectStore.cpp @@ +755,5 @@ > + NS_WARNING("Snappy can't determine uncompressed length!"); > + return NS_ERROR_DOM_INDEXEDDB_UNKNOWN_ERR; > + } > + > + return aBuffer.copy(reinterpret_cast<const uint64_t *>(uncompressed.get()), Please file a bug on avoiding the copy here.

Comment on attachment 575803 [details] [diff] [review] Patch, v1 [not for checkin, missing snappy source] khuey, can you review the build goop here?

Comment on attachment 575803 [details] [diff] [review] Patch, v1 [not for checkin, missing snappy source] Review of attachment 575803 [details] [diff] [review]: ----------------------------------------------------------------- Do we need to add something to toolkit/content/license.html here? r=me on the "build goop stuff" ::: other-licenses/snappy/Makefile.in @@ +41,5 @@ > + > +VPATH = \ > + @srcdir@ \ > + $(topsrcdir)/other-licenses/snappy/src > + $(NULL) VPATH += $(topsrcdir)/other-licenses/snappy/src should be enough

(In reply to Kyle Huey [:khuey] (khuey@mozilla.com) from comment #6) > ::: other-licenses/snappy/Makefile.in > @@ +41,5 @@ > > + > > +VPATH = \ > > + @srcdir@ \ > > + $(topsrcdir)/other-licenses/snappy/src > > + $(NULL) > > VPATH += $(topsrcdir)/other-licenses/snappy/src should be enough Actually, ignore me, I'm being silly.

Backed out of inbound for PGO Linux64 build failures: https://tbpl.mozilla.org/?tree=Mozilla-Inbound&rev=7634808d94af https://tbpl.mozilla.org/php/getParsedLog.php?id=7527408&tree=Mozilla-Inbound { /tools/python/bin/python2.5 /builds/slave/m-in-lnx64-pgo/build/config/pythonpath.py -I../../config /builds/slave/m-in-lnx64-pgo/build/config/expandlibs_exec.py --uselist -- /usr/bin/ccache /tools/gcc-4.5/bin/g++ -fno-rtti -pedantic -Wall -Wpointer-arith -Woverloaded-virtual -Wsynth -Wno-ctor-dtor-privacy -Wno-non-virtual-dtor -Wcast-align -Wno-invalid-offsetof -Wno-variadic-macros -Werror=return-type -Wno-long-long -fno-exceptions -fno-strict-aliasing -std=gnu++0x -pthread -pipe -DNDEBUG -DTRIMMED -g -fprofile-generate -O3 -fomit-frame-pointer -fPIC -shared -Wl,-z,defs -Wl,-h,libxul.so -o libxul.so stdc++compat.i_o nsStaticXULComponents.i_o nsUnicharUtils.i_o nsBidiUtils.i_o nsRDFResource.i_o -lpthread -fprofile-generate -Wl,-rpath-link,/builds/slave/m-in-lnx64-pgo/build/obj-firefox/dist/bin -Wl,-rpath-link,/usr/local/lib ../../toolkit/xre/libxulapp_s.a ../../staticlib/components/libnecko.a ../../staticlib/components/libuconv.a ../../staticlib/components/libi18n.a ../../staticlib/components/libchardet.a ../../staticlib/components/libjar50.a ../../staticlib/components/libstartupcache.a ../../staticlib/components/libpref.a ../../staticlib/components/libhtmlpars.a ../../staticlib/components/libimglib2.a ../../staticlib/components/libgkgfx.a ../../staticlib/components/libgklayout.a ../../staticlib/components/libdocshell.a ../../staticlib/components/libembedcomponents.a ../../staticlib/components/libwebbrwsr.a ../../staticlib/components/libnsappshell.a ../../staticlib/components/libtxmgr.a ../../staticlib/components/libcommandlines.a ../../staticlib/components/libtoolkitcomps.a ../../staticlib/components/libpipboot.a ../../staticlib/components/libpipnss.a ../../staticlib/components/libappcomps.a ../../staticlib/components/libjsreflect.a ../../staticlib/components/libcomposer.a ../../staticlib/components/libjetpack_s.a ../../staticlib/components/libtelemetry.a ../../staticlib/components/libjsdebugger.a ../../staticlib/components/libstoragecomps.a ../../staticlib/components/librdf.a ../../staticlib/components/libwindowds.a ../../staticlib/components/libjsctypes.a ../../staticlib/components/libjsperf.a ../../staticlib/components/libgkplugin.a ../../staticlib/components/libunixproxy.a ../../staticlib/components/libjsd.a ../../staticlib/components/libautoconfig.a ../../staticlib/components/libauth.a ../../staticlib/components/libcookie.a ../../staticlib/components/libpermissions.a ../../staticlib/components/libuniversalchardet.a ../../staticlib/components/libfileview.a ../../staticlib/components/libplaces.a ../../staticlib/components/libtkautocomplete.a ../../staticlib/components/libsatchel.a ../../staticlib/components/libpippki.a ../../staticlib/components/libwidget_gtk2.a ../../staticlib/components/libsystem-pref.a ../../staticlib/components/libimgicon.a ../../staticlib/components/libaccessibility.a ../../staticlib/components/libremoteservice.a ../../staticlib/components/libspellchecker.a ../../staticlib/components/libzipwriter.a ../../staticlib/components/libservices-crypto.a ../../staticlib/libjsipc_s.a ../../staticlib/libdomipc_s.a ../../staticlib/libdomplugins_s.a ../../staticlib/libmozipc_s.a ../../staticlib/libmozipdlgen_s.a ../../staticlib/libipcshell_s.a ../../staticlib/libgfx2d.a ../../staticlib/libgfxipc_s.a ../../staticlib/libhal_s.a ../../staticlib/libxpcom_core.a ../../staticlib/libucvutil_s.a ../../staticlib/libchromium_s.a ../../staticlib/libmozreg_s.a ../../staticlib/libsnappy_s.a ../../staticlib/libgtkxtbin.a ../../staticlib/libthebes.a ../../staticlib/libgl.a ../../staticlib/libycbcr.a ../../staticlib/libangle.a -L../../dist/bin -L../../dist/lib ../../media/libjpeg/libmozjpeg.a ../../media/libpng/libmozpng.a ../../gfx/qcms/libmozqcms.a /builds/slave/m-in-lnx64-pgo/build/obj-firefox/dist/lib/libjs_static.a -L../../dist/bin -L../../dist/lib -lcrmf -lsmime3 -lssl3 -lnss3 -lnssutil3 ../../gfx/cairo/cairo/src/libmozcairo.a ../../gfx/cairo/libpixman/src/libmozlibpixman.a -L/usr/lib64 -lXrender -lfreetype -lfontconfig ../../gfx/harfbuzz/src/libmozharfbuzz.a ../../gfx/ots/src/libmozots.a ../../dist/lib/libmozsqlite3.a ../../modules/zlib/src/libmozz.a -lasound -lm -ldl -lpthread -lrt -L../../dist/bin -L../../dist/lib -L/builds/slave/m-in-lnx64-pgo/build/obj-firefox/dist/lib -lplds4 -lplc4 -lnspr4 -lpthread -ldl ../../dist/lib/libmozalloc.a -L/lib64 -ldbus-glib-1 -ldbus-1 -lglib-2.0 -L/usr/lib64 -lX11 -lXext -L/lib64 -lpangoft2-1.0 -lpangocairo-1.0 -lpango-1.0 -lcairo -lgobject-2.0 -lgmodule-2.0 -ldl -lglib-2.0 -L/lib64 -lgtk-x11-2.0 -latk-1.0 -lgdk-x11-2.0 -lgdk_pixbuf-2.0 -lm -lpangocairo-1.0 -lpango-1.0 -lcairo -lgobject-2.0 -lgmodule-2.0 -ldl -lglib-2.0 -lXt -lgthread-2.0 -lfreetype -ldl -lrt /usr/bin/ld: ../../other-licenses/snappy/snappy.i_o: relocation R_X86_64_PC32 against `std::__throw_bad_cast()' can not be used when making a shared object; recompile with -fPIC /usr/bin/ld: final link failed: Bad value collect2: ld returned 1 exit status make[6]: *** [libxul.so] Error 1 } https://hg.mozilla.org/integration/mozilla-inbound/rev/30d495095e2b

Comment on attachment 575803 [details] [diff] [review] Patch, v1 [not for checkin, missing snappy source] >- // Get the data version. >- nsCOMPtr<mozIStorageStatement> stmt; >- rv = connection->CreateStatement(NS_LITERAL_CSTRING( >- "SELECT dataVersion " >- "FROM database" >- ), getter_AddRefs(stmt)); >+ if (!mForDeletion) { >+ // Get the data version, and maybe vacuum if we upgrade (hopefully because >+ // we figured out a way to save some disk space). >+ PRInt64 dataVersion; >+ rv = GetDataVersion(connection, &dataVersion); >+ NS_ENSURE_SUCCESS(rv, NS_ERROR_DOM_INDEXEDDB_UNKNOWN_ERR); >+ >+ // This logic needs to change next time we change the data version! >+ PR_STATIC_ASSERT(kJSStructuredCloneVersion == 1); >+ PR_STATIC_ASSERT(kInternalDataVersion == 1); >+ >+ bool vacuumNeeded = false; >+ >+ if (dataVersion != kDataVersion && !mForDeletion) { redundant !mForDeletion check ? I'm not checking all your patches :) I'm just updating files in idb patch and found it by chance.

Final patch.

Comment on attachment 579252 [details] [diff] [review] Patch, final Review of attachment 579252 [details] [diff] [review]: ----------------------------------------------------------------- r=me with that fixed ::: dom/indexedDB/OpenDatabaseHelper.cpp @@ +75,5 @@ > +PRInt32 > +MakeSchemaVersion(PRUint32 aMajorSchemaVersion, > + PRUint32 aMinorSchemaVersion) > +{ > + return PRInt32((aMajorSchemaVersion << 0x8) + aMinorSchemaVersion); should be 4, no 0x8 @@ +78,5 @@ > +{ > + return PRInt32((aMajorSchemaVersion << 0x8) + aMinorSchemaVersion); > +} > + > +const PRInt32 kSQLiteSchemaVersion = PRInt32((kMajorSchemaVersion << 0x8) + Same here @@ +84,5 @@ > + > +inline > +PRUint32 GetMajorSchemaVersion(PRInt32 aSchemaVersion) > +{ > + return PRUint32(aSchemaVersion) >> 0x8; And here @@ +1010,5 @@ > NS_ENSURE_SUCCESS(rv, rv); > > + if (schemaVersion > kSQLiteSchemaVersion) { > + NS_WARNING("Unable to open IndexedDB database, schema is too high!"); > + return NS_ERROR_DOM_INDEXEDDB_UNKNOWN_ERR; Can you file a bug on firing a more specific error here @@ +1043,5 @@ > + NS_ENSURE_SUCCESS(rv, NS_ERROR_DOM_INDEXEDDB_UNKNOWN_ERR); > + } > + else { > + // This logic needs to change next time we change the schema! > + PR_STATIC_ASSERT(kSQLiteSchemaVersion == PRInt32((9 << 0x8) + 0)); 4 again @@ +1050,5 @@ > + if (schemaVersion == 4) { > + rv = UpgradeSchemaFrom4To5(connection); > + } > + else if (schemaVersion == 5) { > + rv = UpgradeSchemaFrom5To6(connection); This won't work. Once we've upgraded to 4->5 we should upgrade to 5->6 no? I think you need to just remove the |else| @@ +1094,5 @@ > } > > // Turn on foreign key constraints. > rv = connection->ExecuteSimpleSQL(NS_LITERAL_CSTRING( > "PRAGMA foreign_keys = ON;" Might be worth putting the foreign_keys=OFF at the top of the upgrade code so that we don't have to remember to do that everywhere. Otherwise some upgrade paths which set it would work, while upgrading from newer version might wipe all data

(In reply to Jonas Sicking (:sicking) from comment #13) > should be 4, no 0x8 Blegh. Fixed. > Can you file a bug on firing a more specific error here Do we really care? This isn't likely to be a problem for anyone other than nightly testers... > This won't work. Once we've upgraded to 4->5 we should upgrade to 5->6 no? It's in a loop so this will work fine. > Might be worth putting the foreign_keys=OFF at the top of the upgrade code It's off by default, I'll just remove these.

Filed bug 708158 about the crazy linux pgo failure.

This is needed to fix linux PGO. Apparently it was un-inlining these functions. Thanks to cjones and jlebar for helping me debug.

Comment on attachment 579603 [details] [diff] [review] Fix linux PGO r=me with s/inline/NS_ALWAYS_INLINE/ per discussion.

Backed out in https://hg.mozilla.org/integration/mozilla-inbound/rev/f0f0ec491b9e for Windows PGO build failures. Suspicious as hell that we spent all day yesterday with a "nswindowmediator.cpp(821) : fatal error C1001: An internal error has occurred in the compiler. LINK : fatal error LNK1000: Internal error during IMAGE::BuildImage" failure too, but once we backed out bug 676349 we did go back to being able to build PGO. Twice. Then this landed. We'll see whether this backout also manages to cure it.

(In reply to Phil Ringnalda (:philor) from comment #19) > Backed out in > https://hg.mozilla.org/integration/mozilla-inbound/rev/f0f0ec491b9e for > Windows PGO build failures. Wait, the very next build succeeded, didn't it? https://hg.mozilla.org/integration/mozilla-inbound/rev/76a185935937

Odd. Strictly speaking, no, because those "PGO" tests are from the nightly, rather than a periodic PGO build, but nightlies are PGO'ed, and as far as I know, the only difference between them and periodic PGO is that they always clobber. Possible that I need to clobber the piss out of things, and retrigger dozens of PGO builds.

Clobber set, PGO retriggered on your push, should know in 4-6 hours.

Sadly, clobbering doesn't seem to have been the answer, the second one in https://tbpl.mozilla.org/?tree=Mozilla-Inbound&rev=8941e2b7a0bf says it was a clobber, and still had the same compiler crash.

While the clobber of the bug 676349 push was green, so I don't have the slightest hint of a clue.

Since it doesn't really seem like this patch is the culprit I went ahead and relanded in https://hg.mozilla.org/integration/mozilla-inbound/rev/0dd4e34b2f6f.

And subsequent builds on the original push look fine. Solar wind, perhaps? https://tbpl.mozilla.org/php/getParsedLog.php?id=7811571&tree=Mozilla-Inbound&full=1

snappy.cc doesn't compile under gcc4.3.2 because of extraneous semicolons :-(