proof of concept: assets as references to blobs in indexedDB, not directly as base64 in store

mimecuvalo commented 1 month ago

As I look at LOD holistically and whether we have multiple sources when working locally, I learned that our system used base64 encoding of assets directly. Issue https://github.com/tldraw/tldraw/issues/3728

The motivations and benefits are:

store size: not having a huge base64 blobs injected in room data
perf on loading snapshot: this helps with loading the room data more quickly
multiple sources: furthermore, if we do decide to have multiple sources locally (for each asset), then we won't get a multiplicative effect of even larger JSON blobs that have lots of base64 data in them
encoding/decoding perf: this also saves the (slow) step of having to base64 encode/decode our assets, we can just strictly with work with blobs.

The benefit here is clear! I created an AssetBlobObjectStore which probably needs some thought (again, this is proof-of-concept), so if we decide to move forward, let's discuss how best to integrate that part.

Todo:

[x] decodes video and images
[x] make sure it syncs to other tabs
[x] make sure it syncs to other multiplayer room
[ ] fix tests
[ ] have backup for existing editors instantiated without assetBlobStore

Change Type

[x] sdk — Changes the tldraw SDK
[ ] dotcom — Changes the tldraw.com web app
[ ] docs — Changes to the documentation, examples, or templates.
[ ] vs code — Changes to the vscode plugin
[ ] internal — Does not affect user-facing stuff

[ ] bugfix — Bug fix
[ ] feature — New feature
[x] improvement — Improving existing features
[ ] chore — Updating dependencies, other boring stuff
[ ] galaxy brain — Architectural changes
[ ] tests — Changes to any test code
[ ] tools — Changes to infrastructure, CI, internal scripts, debugging tools, etc.
[ ] dunno — I don't know

Test Plan

Test the shit out of uploading/downloading video/image assets, locally+multiplayer.

[ ] Need to fix current tests and write new ones

Release Notes

Assets: store as reference to blob in indexedDB instead of storing directly as base64 in the snapshot.

vercel[bot] commented 1 month ago

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Updated (UTC)
examples	❌ Failed (Inspect)		May 13, 2024 8:32am

1 Ignored Deployment

SomeHats commented 3 weeks ago

left some other thoughts on this here: https://github.com/tldraw/tldraw/pull/3764#issuecomment-2122516560

my take on this is that I like what we're doing with this, but i'm not sure it belongs quite so core as this makes it. i think blob storage is quite niche - it's really only relevant if you have an editor saving locally in the browser, whereas most people either have a multiplayer room or some sort of other saving mechanism. i certainly don't think it should be the default as this PR makes it.

instead, i would love to see a slightly more general system for making asset storage/retrieval more flexible. what would it look like for this to be build entirely in userland on your asset rewriting hook from #3764, for example?

mimecuvalo commented 3 weeks ago

my take on this is that I like what we're doing with this, but i'm not sure it belongs quite so core as this makes it. i think blob storage is quite niche - it's really only relevant if you have an editor saving locally in the browser, whereas most people either have a multiplayer room or some sort of other saving mechanism. i certainly don't think it should be the default as this PR makes it.

yeah, totally fair to not make it core, definitely would make sense more as a sidecar kind of option.

instead, i would love to see a slightly more general system for making asset storage/retrieval more flexible. what would it look like for this to be build entirely in userland on your asset rewriting hook from https://github.com/tldraw/tldraw/pull/3764, for example?

i see the overlap for sure, but i think in my head they're quite different (as far as storage vs. dynamic retrieval). i'd avoid making a system that tries to address them both perhaps because they're trying to address two different problems. (it's maybe a coincidence that I'm working on both at the same time heh)

let's discuss more tomorrow!

mimecuvalo commented 2 weeks ago

(closed in favor of https://github.com/tldraw/tldraw/pull/3836)

tldraw / tldraw