automerge / automerge-classic

A JSON-like data structure (a CRDT) that can be modified concurrently by different users, and merged again automatically.
http://automerge.org/
MIT License
14.75k stars 466 forks source link

Just store hashes in SyncState rather than changes #341

Closed jeffa5 closed 3 years ago

jeffa5 commented 3 years ago

The changes we store in the SyncState are already stored in the document so we should instead just store the hash and retrieve the change from the document when we require it. This helps to keep the SyncState small.

ept commented 3 years ago

This is not totally ideal because the additional call to decodeChangeMeta is going to unnecessarily recompute the SHA256 hashes of all the changes. I think that can be avoided by rejigging the APIs a bit.

@pvh I wasn't really ready to merge this yet. But we can keep it for now and revisit it later.

Also, we can do still better than storing a list of the hashes of all the changes we've sent: in the common case where we send all changes since lastSync, I think we could just store the heads as an indication that we've sent everything up to those heads. We then only need to keep a list of individual change hashes in the case where we sent a subset of changes. But we can do this after the 1.0-preview release since the SyncState is not a public API, so we can change it.

jeffa5 commented 3 years ago

Yeah, the calls to decodeChangeMeta aren't ideal, in the rust version we have it as a property of the struct so I guess we could have similar here with just computing it on the first time and caching sort of behaviour.

Yep, I think there is still work here but this seemed like a nice simple reduction for now.