Skip to content

C#/Unity SDK - Pre-hash rows and add SmallHashSets #3146

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 14 commits into
base: master
Choose a base branch
from

Conversation

bfops
Copy link
Collaborator

@bfops bfops commented Aug 8, 2025

Description of Changes

Migrating clockworklabs/com.clockworklabs.spacetimedbsdk#318 since we are merging that repo into this one.

Annotate rows with their hashes. This is halfway to the old solution of annotating code with their serialized forms; it speeds up insertion into BTreeIndexes while saving a byte array allocation per-row.
(Wait, why do we hash when inserting into BTreeIndexes, you ask? It's because we use a HashSet to store the multiple rows corresponding to a non-unique key.)

This saves work on the main thread repeatedly hashing values. Once we parallelize message processing, this work will be spread out over multiple threads too.

Also adds a data type SmallHashSet<T> for use in BTreeIndexes. This is a struct that can store at most one element without allocating. This gives dramatic allocation reduction on initial connection -- it seems that most rows in BTreeIndexes don't have any other rows with the same key.
Then I manually specialized this, since it's only used on one type -- PreHashedRow. using SmallHashSetOfPreHashedRow instead of SmallHashSet<PreHashedRow> allows us to inline some methods and pass around fewer pointers while hashing things, which is probably good.

API and ABI breaking changes

None

Expected complexity level and risk

Testing

I will test:

  • blackholio
  • bitcraft

@bfops
Copy link
Collaborator Author

bfops commented Aug 8, 2025

Given the discussion in the original PR, I think maybe we should close this? @rekhoff what do you think?

@bfops bfops added the Do not merge Do not merge PRs with this label without coordinating further label Aug 8, 2025
@bfops bfops marked this pull request as draft August 8, 2025 17:59
@bfops bfops removed the Do not merge Do not merge PRs with this label without coordinating further label Aug 8, 2025
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