Use LZ4 with WASM for volume saving/undo/redo #6652

philippotto · 2022-11-21T14:36:19Z

See #5970 and #6652 (comment) for context and numbers.

URL of deployed dev instance (used for testing):

https://lzwasm.webknossos.xyz/

Steps to test:

brush and use undo/redo
brush and save and reload page
everything should work as usual

Issues:

fixes Speed up undo/redo for volume data (with WASM based lz4) #5970

Updated (unreleased) changelog
Ready for review

frontend/javascripts/libs/utils.ts

philippotto · 2022-11-21T14:44:56Z

frontend/javascripts/test/model/volumetracing/volume_annotation_sampling.spec.ts

@@ -8,7 +9,7 @@ import anyTest from "ava";
 import datasetServerObject from "test/fixtures/dataset_server_object";
 import mockRequire from "mock-require";
 import sinon from "sinon";
-mockRequire.stopAll();


the stopAlls reverted the lz4 mocking (which is necessary since the lz4-wasm module is different for browser and node, unfortunately). it took me a while to figure out that stopAll was the culprit, since it's so inconspicuously in the code. luckily, these statements weren't even necessary.

it's a bit unfortunate that the lz4 mock needs to be at that many places, but I couldn't figure out a better way..

daniel-wer

Wow, extremely impressive improvements! 🤯 Thank you for researching and testing 🙏

Unfortunately, I don't get the mocking yet. Why do files that don't have any direct lz4 usages need to import the lz4 mock? - and wouldn't that also import the mock in production? Why not mock only in the test files where it's needed using mockRequire like we usually do? 🤔

philippotto · 2022-11-22T08:43:01Z

Why do files that don't have any direct lz4 usages need to import the lz4 mock? - and wouldn't that also import the mock in production? Why not mock only in the test files where it's needed using mockRequire like we usually do?

Oh, two non-spec files slipped in there by accident. I corrected these now.

philippotto · 2022-11-22T08:57:40Z

Hmm, the wasm doesn't load on the dev instance -.- I'm looking into it..

philippotto · 2022-11-22T09:16:22Z

Hmm, the wasm doesn't load on the dev instance -.- I'm looking into it..

@fm3 I think the problem is that the backend serves the *.wasm file with content-type: application/octet-stream. However, it needs to be application/wasm. Could you have a look for how to adapt this?

philippotto · 2022-11-22T12:48:38Z

@daniel-wer please have another look :) now, the deployment at https://lzwasm.webknossos.xyz should work, too.

daniel-wer

LGTM with or without my suggestion 👍

During testing of the dev instance, the performance improvement compared to master didn't appear that large. Is that the case for you too? Still, there is an improvement :)

daniel-wer · 2022-11-22T16:10:56Z

frontend/javascripts/test/backend-snapshot-tests/annotations.e2e.ts

@@ -1,3 +1,4 @@
+import "test/mocks/lz4";


Instead of importing the mock in every test file, I think it should work and would be more foolproof to import it once in frontend/javascripts/test/enzyme/e2e-setup.ts or do you think the import order could be problematic, then? 🤔

Sorry to comment on the mocking again. It's no big deal if you leave it as-is.

…-wasm

…stance

…n dev instance" This reverts commit c86c647.

philippotto · 2022-11-23T10:01:43Z

During testing of the dev instance, the performance improvement compared to master didn't appear that large. Is that the case for you too? Still, there is an improvement :)

For the record: I measured the performance difference on the dev instance and unfortunately couldn't reproduce the major difference which I observed on localhost before. It seems like the old lz4-js library is way faster (and almost on-par with the wasm library) when being compiled by webpack with production=true.

This observation goes a bit against the benchmark cited here, but I assume it can be explained by: (a) too small inputs or (b) the input is "too easy" to compress which is why the wasm library cannot really show its strengths.

I think, we should proceed with merging this PR nevertheless, since (a) (de)compression will be much faster on localhost (i.e., during development) and (b) the old library doesn't seem to be really maintained, anymore (e.g., the latest commit is from 2018 but the latest release is from 2016).

…server)

philippotto · 2022-11-23T10:39:27Z

I also bumped the pako library (use for gzip compression when sending save requests) because the version we used dates back to 2015 🙈 The new version is two weeks old. I couldn't see measurable speed improvements, but it shouldn't hurt nonetheless.

daniel-wer · 2022-11-23T13:01:05Z

Thank you for re-measuring! I also think we should still proceed with merging this PR 👍

…cing * 'master' of github.com:scalableminds/webknossos: (23 commits) Guard against invalid-mag bucket volume save actions (#6660) Add OIDC authentication (#6534) Add second (non-admin) default user to "initial data" trigger (#6666) Fix and improve miscellaneous things in Folders tab (#6674) Improves mag and voxelSize inferral for remote datasets (#6670) Squashed commit of the following: Fix import of N5 datasets by adding n5 schema to frontend validation (#6668) Virtual Folder Structure for Datasets (#6591) Ability to recover from webGL context loss (#6663) Fix assertion that referenced teams cannot be deleted (#6664) fix task summary with pending jobs (#6662) Only allow taskTypeId as parameter in task creation (#6640) Release 22.12.0 (#6661) Create Annotation From View Mode: Keep Activated Mapping (#6647) Workaround to avoid false-positive version warning (#6656) Fix WK-lib download snippet (#6605) Use LZ4 with WASM for volume saving/undo/redo (#6652) Fix version number extraction when building docker image (#6655) Pass mapping name to precompute meshes (#6651) Deduplicate bboxes when importing NML (#6648) ...

philippotto added 8 commits November 21, 2022 12:04

fix conversion of NaN to big int

d029734

use lz4-wasm for compression/decompression of buckets (for undo/redo)

b0e4ed8

also use lz4-wasm for compression of buckets (for saving)

ec39a18

clean up lz-wasm usage

d8c79b0

fix tests by using different lz4 library there via mocking

c4bb49b

more mocks

1724ea2

update changelog

7e60e85

more mocks

ddf9f5c

philippotto self-assigned this Nov 21, 2022

philippotto requested a review from daniel-wer November 21, 2022 14:41

philippotto commented Nov 21, 2022

View reviewed changes

add missing file

6384566

daniel-wer reviewed Nov 21, 2022

View reviewed changes

philippotto added 3 commits November 22, 2022 08:47

fix accidental mock of non-spec files

b6d7f24

fix accidental mock of non-spec files

194c0b8

also mock in e2e tests

2b9cc24

fm3 added 2 commits November 22, 2022 11:48

add wasm mime type

f96cb64

extract slick conf

3a0abd8

daniel-wer approved these changes Nov 22, 2022

View reviewed changes

philippotto added 3 commits November 23, 2022 09:54

Merge branch 'master' of github.com:scalableminds/webknossos into lz4…

df53044

…-wasm

extract lz4 mock in e2e specs away using e2e-setup

062f846

re-add slow lz4js library to conduct performance comparison on dev in…

c86c647

…stance

philippotto force-pushed the lz4-wasm branch from 2f5d827 to c86c647 Compare November 23, 2022 09:06

philippotto added 2 commits November 23, 2022 10:52

Revert "re-add slow lz4js library to conduct performance comparison o…

738d29f

…n dev instance" This reverts commit c86c647.

tune down changelog entry

7f19d04

philippotto changed the title ~~Speed up volume saving/undo/redo by using LZ4 with WASM~~ Use LZ4 with WASM for volume saving/undo/redo Nov 23, 2022

bump pako version (used for gzip compression when sending buckets to …

dad8f0d

…server)

philippotto mentioned this pull request Nov 23, 2022

Workaround to avoid false-positive version warning #6656

Merged

2 tasks

Merge branch 'master' into lz4-wasm

f581e79

philippotto enabled auto-merge (squash) November 23, 2022 13:05

philippotto merged commit 7d8ca61 into master Nov 23, 2022

philippotto deleted the lz4-wasm branch November 23, 2022 13:23

fm3 mentioned this pull request Nov 24, 2022

Add OIDC authentication #6534

Merged

8 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use LZ4 with WASM for volume saving/undo/redo #6652

Use LZ4 with WASM for volume saving/undo/redo #6652

philippotto commented Nov 21, 2022 •

edited

Loading

philippotto Nov 21, 2022

daniel-wer left a comment •

edited

Loading

philippotto commented Nov 22, 2022

philippotto commented Nov 22, 2022

philippotto commented Nov 22, 2022

philippotto commented Nov 22, 2022

daniel-wer left a comment

daniel-wer Nov 22, 2022

philippotto commented Nov 23, 2022

philippotto commented Nov 23, 2022

daniel-wer commented Nov 23, 2022

Use LZ4 with WASM for volume saving/undo/redo #6652

Use LZ4 with WASM for volume saving/undo/redo #6652

Conversation

philippotto commented Nov 21, 2022 • edited Loading

URL of deployed dev instance (used for testing):

Steps to test:

Issues:

philippotto Nov 21, 2022

Choose a reason for hiding this comment

daniel-wer left a comment • edited Loading

Choose a reason for hiding this comment

philippotto commented Nov 22, 2022

philippotto commented Nov 22, 2022

philippotto commented Nov 22, 2022

philippotto commented Nov 22, 2022

daniel-wer left a comment

Choose a reason for hiding this comment

daniel-wer Nov 22, 2022

Choose a reason for hiding this comment

philippotto commented Nov 23, 2022

philippotto commented Nov 23, 2022

daniel-wer commented Nov 23, 2022

philippotto commented Nov 21, 2022 •

edited

Loading

daniel-wer left a comment •

edited

Loading