Closed Bug 1340901 Opened 8 years ago Closed 4 years ago

Update Snappy library to 1.1.8 version

Categories

(Core :: Storage: IndexedDB, enhancement, P3)

Product:
Core
Component:
Storage: IndexedDB
Type:
enhancement

Tracking

()

Status:
VERIFIED FIXED
Milestone:
87 Branch
Tracking Flags:
Tracking Status
firefox87 --- verified

People

(Reporter: Virtual, Assigned: sg)

References

(
URL
)

Details

(Keywords: nightly-community, perf, Whiteboard: DWS_NEXT)

Attachments

(1 file, 1 obsolete file)

Bug 1340901 - Update Snappy to version 1.1.8. r=#dom-workers-and-storage
(deleted), text/x-phabricator-request
Details
https://github.com/google/snappy/releases > Snappy 1.1.4 changelog: > Fix a 1% performance regression when snappy is used in PIE executables. > Improve compression performance by 5%. > Improve decompression performance by 20%.
Flags: needinfo?(jvarga)
Has Regression Range: --- → irrelevant
Has STR: --- → irrelevant
status-firefox54: affected → ---
This affects Cache API as well. If the compressed format is not fully backward compatible please be sure to bump the schema version in dom/cache/DBSchema.cpp.
:Virtual, is the new compressed format fully backward compatible ?
Flags: needinfo?(jvarga) → needinfo?(Virtual)
By fully backward compatible I mean that compressed data saved in older Firefox can be successfully decompressed in new Firefox (with snappy 1.1.4) and also that compressed data stored in new Firefox can be successfully decompressed in older Firefox.
Priority: -- → P3
> and also that compressed data stored in new Firefox can be successfully decompressed in older Firefox. We just need to bump the various storage schema version numbers if this isn't true, right?
(In reply to Ben Kelly [not reviewing due to deadline][:bkelly] from comment #4) > > and also that compressed data stored in new Firefox can be successfully decompressed in older Firefox. > > We just need to bump the various storage schema version numbers if this > isn't true, right? Yeah, but you know, it means that users will have bad time again if they switch between channels. I would rather wait to do it as part of a bigger "upgrade" if people don't mind.
(In reply to Jan Varga [:janv] from comment #2) > :Virtual, is the new compressed format fully backward compatible ? Unfortunately, I don't know and I didn't saw any information that it is or is not backward compatible. (In reply to Jan Varga [:janv] from comment #5) > (In reply to Ben Kelly [not reviewing due to deadline][:bkelly] from comment > #4) > > > and also that compressed data stored in new Firefox can be successfully decompressed in older Firefox. > > > > We just need to bump the various storage schema version numbers if this > > isn't true, right? > > Yeah, but you know, it means that users will have bad time again if they > switch between channels. > I would rather wait to do it as part of a bigger "upgrade" if people don't > mind. Users will already have bad time, if they will want to downgrade to older Firefox channel, it's due to bug #977177 comment #78, so updating Snappy library in Firefox 55 won't be increasing obstacles.
Flags: needinfo?(Virtual)
(In reply to Virtual_ManPL [:Virtual] - (please needinfo? me - so I will see your comment/reply/question/etc.) from comment #6) > (In reply to Jan Varga [:janv] from comment #2) > > :Virtual, is the new compressed format fully backward compatible ? > > Unfortunately, I don't know and I didn't saw any information that it is or > is not backward compatible. In the past the snappy folks have been good about mentioning back compat problems in their release notes. If it doesn't call it out, then it should be compatible. We should test, though.
@ Jan Varga [:janv] - Per comment #6, what do you think about updating Snappy library to 1.1.4 version in Firefox 55?
Flags: needinfo?(jvarga)
FYI - Snappy 1.1.5 was released > Add CMake build support. The autoconf build support is now deprecated, and will be removed in the next release. > Add AppVeyor configuration, for Windows CI coverage. > Small performance improvement on little-endian PowerPC. > Small performance improvement on LLVM with position-independent executables. > Fix a few issues with various build environments.
Summary: Update Snappy library to 1.1.4 version → Update Snappy library to 1.1.5 version
FYI - Snappy 1.1.6 was released > This is a re-release of v1.1.5 with proper SONAME / SOVERSION values. > SONAME / SOVERSION errors will manifest as the dynamic library loader complaining > that it cannot find snappy's shared library file (libsnappy.so / libsnappy.dylib), > or that the library it found does not have the required version.
Summary: Update Snappy library to 1.1.5 version → Update Snappy library to 1.1.6 version
FYI - Snappy 1.1.7 was released > Improved CMake build support for 64-bit Linux distributions. > MSVC builds now use MSVC-specific intrinsics that map to clzll. > ARM64 (AArch64) builds use the code paths optimized for 64-bit processors.
Summary: Update Snappy library to 1.1.6 version → Update Snappy library to 1.1.7 version
Clearing old NI but adding to DWS_NEXT
Flags: needinfo?(jvarga)
Whiteboard: DWS_NEXT

(In reply to Jan Varga [:janv] from comment #12)

try push with snappy 1.1.7:
https://treeherder.mozilla.org/#/
jobs?repo=try&revision=1c8749e72cea9e7ce2106739c4af714acc8a61a3

Can this land or waiting for something?

Flags: needinfo?(jvarga)

Unfortunately this is currently not a high priority, but would be great to have once we decide to implement internal compression of data in LocalStorage.

Blocks: 1540402
Flags: needinfo?(jvarga)
Blocks: 1513881
No longer blocks: 1513881, 1540402
Assignee: nobody → ssengupta

The mentioned snappy 1.1.7 release is already two years old now, and in the master there have been several improvements since. One notable performance improvement is https://github.com/google/snappy/commit/4aba5426d47af960939d85d6f74c0a6ac7124887.

It will be really nice to finally update snappy in Firefox, as last update was done 3 years ago in bug #768074 by Jan Varga [:janv], and performance boost looks really impressive, at least on paper in change-log. What's more, there is no need to be compatible with older versions of Firefox, as downgrading isn't supported now for some time. Updating to 1.1.7 or even to master branch with latest patches will be nice too.

@ Jan Varga [:janv] - What do you think about it? Also, is it no needed anymore for bug #1513881 and bug #1540402?

Flags: needinfo?(jvarga)

Yeah, Subhamoy already provided an updated patch and I already started reviewing it.

Flags: needinfo?(jvarga)

Asked the Snappy team if they would like to make a release soon: https://groups.google.com/forum/#!topic/snappy-compression/4Nq8HCF85sk

Attachment #9115130 - Attachment description: Bug 1340901 - Snappy updated to master commit f5acee9 r=#dom-workers-and-storage → Bug 1340901 - P2 - Snappy updated to version 1.1.8 r=#dom-workers-and-storage
Summary: Update Snappy library to 1.1.7 version → Update Snappy library to 1.1.8 version
Attachment #9122053 - Attachment description: Bug 1340901 - P1 - Added XPCShell test for foward compatibility to Snappy 1.1.8 r=#dom-workers-and-storage → Bug 1340901 - P1 - Added XPCShell test for forward compatibility to Snappy 1.1.8 r=#dom-workers-and-storage

Can this land or some modification is still needed?

Flags: needinfo?(ssengupta)
Severity: major → S2
Flags: needinfo?(ssengupta)

I cancelled the needinfo purely by mistake as I was setting severity to this task. Truly sorry for that.
This cannot land, because there is a test missing, which I did not get around to writing yet.
Hopefully I can get around to it soon :)

Awesome! No problem. I was pinging to hope it's not forgotten.

Assignee: subhamoy → sgiesecke

Comment on attachment 9122053 [details]
Bug 1340901 - P1 - Added XPCShell test for forward compatibility to Snappy 1.1.8 r=#dom-workers-and-storage

Revision D60516 was moved to bug 1675211. Setting attachment 9122053 [details] to obsolete.

Attachment #9122053 - Attachment is obsolete: true
Attachment #9115130 - Attachment description: Bug 1340901 - P2 - Snappy updated to version 1.1.8 r=#dom-workers-and-storage → Bug 1340901 - Update Snappy version 1.1.8. r=#dom-workers-and-storage
Attachment #9115130 - Attachment description: Bug 1340901 - Update Snappy version 1.1.8. r=#dom-workers-and-storage → Bug 1340901 - Update Snappy to version 1.1.8. r=#dom-workers-and-storage
Pushed by sgiesecke@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/2252ab087d80 Update Snappy to version 1.1.8. r=dom-workers-and-storage-reviewers,asuth

https://hg.mozilla.org/mozilla-central/rev/2252ab087d80

Status: ASSIGNED → RESOLVED
Closed: 4 years ago
status-firefox87: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → 87 Branch
Regressions: 1691957
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: