Skip to content
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

packages/gossiplog - implement isAncestor using branch index #304

Closed
wants to merge 49 commits into from

Conversation

rjwebb
Copy link
Contributor

@rjwebb rjwebb commented Jun 20, 2024

This PR replaces the existing implementation of isAncestor in AbstractGossipLog with one that uses the new $branch_merges table and branch field in the $messages table. This lets us determine if a message is an ancestor of another message without looking at every single message in turn - it instead considers each branch. This should be faster for situations when we have long "chains" of messages.

To do:

  • Implement isAncestor
  • Decide whether to keep or delete the getAncestors function
  • Remove AncestorIndex
  • Remove the indexAncestors constructor option from AbstractGossipLog
  • Rename ModelsInit to ModelSchema
  • Remove the indexHistory setting

How has this been tested?

  • CI tests pass
  • Tested with example-chat (including login, all signers, and exchanging messages)
  • Tested with @canvas-js/test-network: (optional)

Does this contain any breaking changes to external interfaces?

  • Contract interfaces
  • Core interface
  • CLI
  • Data storage formats, including IndexedDB, SQLite, or filesystem storage (will this break existing apps?)

rjwebb added 30 commits June 18, 2024 11:45
…rjwebb/implement-is-ancestor-using-branch-table
@rjwebb rjwebb marked this pull request as ready for review June 21, 2024 14:08
@rjwebb rjwebb changed the base branch from rjwebb/add-child-index-and-branches-to-gossiplog to main June 24, 2024 14:39
packages/gossiplog/src/AbstractGossipLog.ts Show resolved Hide resolved
packages/gossiplog/src/AbstractGossipLog.ts Outdated Show resolved Hide resolved
packages/gossiplog/src/BranchMergeIndex.ts Outdated Show resolved Hide resolved
@rjwebb
Copy link
Contributor Author

rjwebb commented Jul 3, 2024

so i tested the "branch merge" based implementation of isAncestor with the existing ancestor index. the samples aren't random - we just call isAncestor for a message with i and i + 1 or 10 or 100 etc, for i from 0 to 99

what we can see here is that the existing AncestorIndex implementation (which uses the clever logarithmic tree data structure that @joeltg thought of) is much much faster and has sub-linear time complexity.

with branch merge based isAncestor implementation:
Clock offset 1: 0.514ms, n: 100 samples
Clock offset 10: 0.608ms, n: 100 samples
Clock offset 100: 1.630ms, n: 100 samples
Clock offset 1000: 16.774ms, n: 100 samples
Clock offset 10000: 144.366ms, n: 100 samples

with the existing AncestorIndex:
Clock offset 1: 0.043ms, n: 100 samples
Clock offset 10: 0.047ms, n: 100 samples
Clock offset 100: 0.065ms, n: 100 samples
Clock offset 1000: 0.081ms, n: 100 samples
Clock offset 10000: 0.163ms, n: 100 samples

@rjwebb
Copy link
Contributor Author

rjwebb commented Jul 8, 2024

We're going to keep the existing ancestor index as it is faster when dealing with large causal log trees.

@rjwebb rjwebb closed this Jul 8, 2024
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