Skip to content

Improve performance of row ids - #180

Merged
jsdt merged 17 commits into
mainfrom
jsdt/indexing-perf
Jul 14, 2025
Merged

jsdt merged 17 commits into
mainfrom
jsdt/indexing-perf

Conversation

@jsdt

@jsdt jsdt commented Apr 25, 2025 •

Copy link
Copy Markdown
Contributor

Description of Changes

This makes a few performance improvements. It is pretty similar to #179. The key changes are:

  • This updates how rowIds are created. For tables with a primary key, it is the primary key value. Otherwise it is a base64 encoding of the whole row. Previously the rowId was always a string representation of the entire row, even if we had a primary key.
  • This gets rid of the very slow OperationsMap, replacing it with a standard Map. This is done by defining a way to turn any AlgebraicValue into a primitive that can be used as a Map key. This conversion is basically free for any primary keys, since they have primitive values.

This also has the fix of push each new operation instead of using the spread syntax (similar to the fix of using concat that was in #179).

Since I was already making some changes in the TableCache, I added some more context to log messages, so hopefully any future bug reports will be easier to look into.

These changes also give us a way to make find efficient, but I didn't put it in this PR.

Requires SpacetimeDB PRs

Requires this codegen change clockworklabs/SpacetimeDB#2671, so we need to wait for the next release.

I'm going to leave this as a draft until we release a version with the required codegen changes.

Once we get this codegen change released, which adds the CLI version to the generated typescript code. I can add an assertion in the client that the generated code came from a compatible version.

Testing

There are some existing tests in db_connection.test.ts that cover parsing update messages, and this PR adds table_cache.test.ts, which has testing for applying row operations to the table cache (and the resulting callbacks).

@changeset-bot

changeset-bot Bot commented Apr 25, 2025 •

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: e67c263

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@pkg-pr-new

pkg-pr-new Bot commented Apr 25, 2025 •

Copy link
Copy Markdown

Open in StackBlitz

npm i https://pkg.pr.new/@clockworklabs/spacetimedb-sdk@180

commit: e67c263

@jsdt
jsdt marked this pull request as ready for review May 1, 2025 16:57
@jsdt
jsdt marked this pull request as draft May 1, 2025 16:59
@jsdt
jsdt requested a review from JulienLavocat May 5, 2025 15:57

@JulienLavocat JulienLavocat left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the reason to add base64-js? Because we need to support both browsers and non-browser platforms like Node.Js ?

Comment thread packages/sdk/src/db_connection_impl.ts Outdated
Comment thread packages/sdk/src/db_connection_impl.ts
Comment thread packages/sdk/src/spacetime_module.ts Outdated
Comment thread packages/sdk/src/table_cache.ts
Comment thread packages/test-app/server/Cargo.toml Outdated
@jsdt

jsdt commented May 6, 2025

Copy link
Copy Markdown
Contributor Author

What is the reason to add base64-js? Because we need to support both browsers and non-browser platforms like Node.Js ?

Yes, that is why I added it.

@JulienLavocat

Copy link
Copy Markdown
Contributor

LGTM

@jsdt
jsdt marked this pull request as ready for review July 14, 2025 17:05
@jsdt
jsdt merged commit d808e41 into main Jul 14, 2025
@jsdt
jsdt deleted the jsdt/indexing-perf branch July 14, 2025 17:09
bfops pushed a commit that referenced this pull request Jul 29, 2025
* Get it working with base64 encoded row ids, and start adding the better way for indexing.

* Use concat to avoid creating too many arguments for big updates.

* Use primitives as rowIds

* Remove OperationsMap

* Add some table cache tests and regen stuff.

* Cleanup

* revert accidental cargo change.

* Add semver parsing.

* Add a minimum version check.

* Use qmark syntax.

* Regen quickstart-chat bindings with 1.2.0 cli

* Bump minimum codegen version.

* Regen test app with 1.2
@bfops bfops mentioned this pull request Aug 7, 2025
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants