Snappy r59 enables the use of unaligned loads and stores for ARM-based architectures where they are available (ARMv7 and higher). This gives a significant speed boost on ARM, both for compression and decompression. It should not affect x86 at all. https://code.google.com/p/snappy/source/detail?r=59

Fennec's and B2G's IndexedDB performance may benefit from new ARM optimizations in Snappy r59.

Chris, please measure the performance difference and renom when you have data.

Dietrich, B2G may be interested in this possible performance improvement for IndexedDB. We compress IndexedDB data using Google's Snappy compression library, but our tree's snapshot is missing some recent Snappy optimizations for ARMv7. Some of their microbenchmarks improved ~20-30%.

Mlee, can you add this to the backlog? It might be worth having someone from the perf team spend some time seeing what the on-device benefits are.

I haven't had a chance to investigate but we'll need to be very careful that the improvements are backwards-compatible with the previous version that we ship today. Otherwise we will corrupt everyone's previously-stored data...

I believe this is backwards compatible. They increased the max block size from 32kb to 64kb, but this only prevents old decoders from reading from the new compressor. We should be fine opening files compressed with the older decoder. Since I am going to use this for the ServiceWorker Cache, I'll take on updating the snappy lib.

Not going to get to this any time soon.

Comment on attachment 8802694 [details] [diff] [review] patch Review of attachment 8802694 [details] [diff] [review]: ----------------------------------------------------------------- When I spoke to bent about this way back when he suggested we should add a test that verifies IDB/Cache values stored in the old snappy can be read in the new snappy. Do you plan to add such a test?

So it seems there was only the initial landing (bug 703660, firefox 11) and then an update to r56 (bug 715113, firefox 12). We have some tests in dom/indexedDB/test/unit which restore profiles w/ IDB data created in older releases of firefox, so I guess if the tests still pass with the patch in this bug, the latest version of snappy must be backwards-compatible with r56 at least.

(In reply to Ben Kelly [:bkelly] from comment #10) > Comment on attachment 8802694 [details] [diff] [review] > patch > > Review of attachment 8802694 [details] [diff] [review]: > ----------------------------------------------------------------- > > When I spoke to bent about this way back when he suggested we should add a > test that verifies IDB/Cache values stored in the old snappy can be read in > the new snappy. Do you plan to add such a test? See my comment 12. For example there's now dom/indexedDB/test/unit/test_schema18upgrade.js which restores a firefox 18 profile.

Ben, do you want me to add a test that restores a FF 11 profile ? If that's not enough, what else is needed ?

Comment on attachment 8802694 [details] [diff] [review] patch Review of attachment 8802694 [details] [diff] [review]: ----------------------------------------------------------------- Sorry, just lost track of this. Looks good to me, but can you verify that this code is exercised in that test? Maybe add a printf in the new snappy code and see that it runs. I seem to recall IDB only uses snappy for certain types of values and its not clear to me if that test includes those values.

I'll double check, but I know for sure that IDB compresses entire structured clone data. Thanks for the review.

I added a dedicated test for this.

(In reply to Jan Varga [:janv] from comment #16) > I'll double check, but I know for sure that IDB compresses entire structured > clone data. > Thanks for the review. I added a dedicated test and I also added printfs to RawCompress and RawUncompress. I saw the RawUncompress during the test, so we should be ok.

Pushed by jvarga@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/0cbca09be546 Update Snappy library to revision >= r59 to get ARM optimizations; r=bkelly

(In reply to Ben Kelly [:bkelly] from comment #6) > I believe this is backwards compatible. They increased the max block size > from 32kb to 64kb, but this only prevents old decoders from reading from the > new compressor. We should be fine opening files compressed with the older > decoder. > > Since I am going to use this for the ServiceWorker Cache, I'll take on > updating the snappy lib. Oh, shouldn't we increase the schema version in dom/cache/DBSchema.cpp ? I'm increasing schema version for IDB in bug 964561 which is about to land.

Comment on attachment 8804191 [details] [diff] [review] A follow-up fix. Increase schema version of DOM cache databases; r =bkelly Review of attachment 8804191 [details] [diff] [review]: ----------------------------------------------------------------- This is to prevent a Cache with the data serialized using the newer snappy from trying to be opened in a firefox that only understands the older version? That seems reasonable. Thanks.

Pushed by jvarga@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/91152c013711 A follow-up fix. Increase schema version of DOM cache databases to prevent a Cache with the data serialized using the newer snappy from trying to be opened in a firefox that only understands the older version; r=bkelly

(In reply to Ben Kelly [:bkelly] from comment #23) > Comment on attachment 8804191 [details] [diff] [review] > A follow-up fix. Increase schema version of DOM cache databases; r =bkelly > > Review of attachment 8804191 [details] [diff] [review]: > ----------------------------------------------------------------- > > This is to prevent a Cache with the data serialized using the newer snappy > from trying to be opened in a firefox that only understands the older > version? That seems reasonable. Thanks. Exactly.