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.

Sign up for GitHub

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

Jump to bottom

Remove node from index during GC #913

Merged
merged 3 commits into from
Jul 4, 2024
Merged

Remove node from index during GC #913

merged 3 commits into from
Jul 4, 2024

Conversation

raararaara
Copy link
Contributor

@raararaara raararaara commented Jul 4, 2024 •
edited by coderabbitai bot
Loading

What this PR does / why we need it:

Remove node from indexes during GC

While improving GC structure, we missed removing nodes from internal
indexes of Text when purging tombstones.

This commit fixes #866.

Which issue(s) this PR fixes:

Addresses #914

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

Additional documentation:

Checklist:

  • Added relevant tests or not required
  • Didn't break anything

Summary by CodeRabbit

  • New Features

    • Added new methods TreeByIndex() and TreeByID() to facilitate debugging.

  • Bug Fixes

    • Improved garbage collection to ensure consistent deletion of nodes in internal data structures.

  • Tests

    • Introduced a new test case for validating node purges during garbage collection.
    • Added assertions to verify text weight consistency in documents.

@coderabbitai coderabbitai
Copy link

coderabbitai bot commented Jul 4, 2024 •
edited
Loading

Walkthrough

The changes primarily enhance the RGATreeSplit struct's garbage collection functionality by ensuring nodes are correctly deleted from two index trees. Additionally, debugging capabilities are improved by exposing tree structures through new methods in the Text struct. New tests validate the correct behavior of these modifications.

Changes

Files Change Summary
pkg/document/crdt/rga_tree_split.go Modified Purge method to delete nodes from treeByIndex and treeByID trees before adjusting linked list.
pkg/document/crdt/text.go Added imports for llrb and splay. Introduced TreeByIndex and TreeByID methods for debugging.
pkg/document/document_test.go Added purge node from index during GC test within TestDocument to validate garbage collection functionality.
test/integration/text_test.go Added assertions to check the text weight using TreeByIndex.

Assessment Against Linked Issues

Objective (Issue) Addressed Explanation
Apply GCPair to TreeNode, TextNode (#866) ✅
Synchronization fails when editing including Undo In the CodeMirror example (#914) ✅

Poem

In the land of code where data flows,
Trees now sway, in perfect rows.
Nodes purged, with precision and care,
Debugging made easy, solutions to share. 🌳
With tests in place, the system's bright,
Sync is smoother, all day and night. ✨

Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

Share
Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai generate interesting stats about this repository and render them as a table.
    • @coderabbitai show all the console.log statements in this repository.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (invoked as PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@codecov Codecov
Copy link

codecov bot commented Jul 4, 2024 •
edited
Loading

Codecov Report

Attention: Patch coverage is 42.85714% with 4 lines in your changes missing coverage. Please review.

Project coverage is 50.73%. Comparing base (58d45c6) to head (7031160).

Files Patch % Lines
pkg/document/crdt/text.go 0.00% 4 Missing ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##             main     #913      +/-   ##
==========================================
- Coverage   50.74%   50.73%   -0.01%     
==========================================
  Files          70       70              
  Lines       10551    10558       +7     
==========================================
+ Hits         5354     5357       +3     
- Misses       4666     4670       +4     
  Partials      531      531

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

hackerwins
hackerwins reviewed Jul 4, 2024
View reviewed changes
pkg/document/document_test.go Outdated Show resolved Hide resolved
pkg/document/document_test.go Outdated Show resolved Hide resolved
@hackerwins hackerwins marked this pull request as ready for review July 4, 2024 05:15
github-actions[bot]
github-actions bot reviewed Jul 4, 2024
View reviewed changes
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Go Benchmark

Benchmark suite Current: 7031160 Previous: 3dfcc78 Ratio
BenchmarkDocument/constructor_test 1551 ns/op 1337 B/op 24 allocs/op 1537 ns/op 1337 B/op 24 allocs/op 1.01
BenchmarkDocument/constructor_test - ns/op 1551 ns/op 1537 ns/op 1.01
BenchmarkDocument/constructor_test - B/op 1337 B/op 1337 B/op 1
BenchmarkDocument/constructor_test - allocs/op 24 allocs/op 24 allocs/op 1
BenchmarkDocument/status_test 939.7 ns/op 1305 B/op 22 allocs/op 969.4 ns/op 1305 B/op 22 allocs/op 0.97
BenchmarkDocument/status_test - ns/op 939.7 ns/op 969.4 ns/op 0.97
BenchmarkDocument/status_test - B/op 1305 B/op 1305 B/op 1
BenchmarkDocument/status_test - allocs/op 22 allocs/op 22 allocs/op 1
BenchmarkDocument/equals_test 8758 ns/op 7273 B/op 132 allocs/op 8824 ns/op 7273 B/op 132 allocs/op 0.99
BenchmarkDocument/equals_test - ns/op 8758 ns/op 8824 ns/op 0.99
BenchmarkDocument/equals_test - B/op 7273 B/op 7273 B/op 1
BenchmarkDocument/equals_test - allocs/op 132 allocs/op 132 allocs/op 1
BenchmarkDocument/nested_update_test 16716 ns/op 12139 B/op 262 allocs/op 16691 ns/op 12139 B/op 262 allocs/op 1.00
BenchmarkDocument/nested_update_test - ns/op 16716 ns/op 16691 ns/op 1.00
BenchmarkDocument/nested_update_test - B/op 12139 B/op 12139 B/op 1
BenchmarkDocument/nested_update_test - allocs/op 262 allocs/op 262 allocs/op 1
BenchmarkDocument/delete_test 22818 ns/op 15363 B/op 341 allocs/op 22486 ns/op 15364 B/op 341 allocs/op 1.01
BenchmarkDocument/delete_test - ns/op 22818 ns/op 22486 ns/op 1.01
BenchmarkDocument/delete_test - B/op 15363 B/op 15364 B/op 1.00
BenchmarkDocument/delete_test - allocs/op 341 allocs/op 341 allocs/op 1
BenchmarkDocument/object_test 8613 ns/op 6817 B/op 120 allocs/op 8584 ns/op 6817 B/op 120 allocs/op 1.00
BenchmarkDocument/object_test - ns/op 8613 ns/op 8584 ns/op 1.00
BenchmarkDocument/object_test - B/op 6817 B/op 6817 B/op 1
BenchmarkDocument/object_test - allocs/op 120 allocs/op 120 allocs/op 1
BenchmarkDocument/array_test 28926 ns/op 11947 B/op 276 allocs/op 28904 ns/op 11947 B/op 276 allocs/op 1.00
BenchmarkDocument/array_test - ns/op 28926 ns/op 28904 ns/op 1.00
BenchmarkDocument/array_test - B/op 11947 B/op 11947 B/op 1
BenchmarkDocument/array_test - allocs/op 276 allocs/op 276 allocs/op 1
BenchmarkDocument/text_test 30565 ns/op 14716 B/op 469 allocs/op 30495 ns/op 14715 B/op 469 allocs/op 1.00
BenchmarkDocument/text_test - ns/op 30565 ns/op 30495 ns/op 1.00
BenchmarkDocument/text_test - B/op 14716 B/op 14715 B/op 1.00
BenchmarkDocument/text_test - allocs/op 469 allocs/op 469 allocs/op 1
BenchmarkDocument/text_composition_test 28873 ns/op 18422 B/op 484 allocs/op 28786 ns/op 18422 B/op 484 allocs/op 1.00
BenchmarkDocument/text_composition_test - ns/op 28873 ns/op 28786 ns/op 1.00
BenchmarkDocument/text_composition_test - B/op 18422 B/op 18422 B/op 1
BenchmarkDocument/text_composition_test - allocs/op 484 allocs/op 484 allocs/op 1
BenchmarkDocument/rich_text_test 81221 ns/op 38476 B/op 1148 allocs/op 81189 ns/op 38477 B/op 1148 allocs/op 1.00
BenchmarkDocument/rich_text_test - ns/op 81221 ns/op 81189 ns/op 1.00
BenchmarkDocument/rich_text_test - B/op 38476 B/op 38477 B/op 1.00
BenchmarkDocument/rich_text_test - allocs/op 1148 allocs/op 1148 allocs/op 1
BenchmarkDocument/counter_test 17357 ns/op 10722 B/op 244 allocs/op 17384 ns/op 10722 B/op 244 allocs/op 1.00
BenchmarkDocument/counter_test - ns/op 17357 ns/op 17384 ns/op 1.00
BenchmarkDocument/counter_test - B/op 10722 B/op 10722 B/op 1
BenchmarkDocument/counter_test - allocs/op 244 allocs/op 244 allocs/op 1
BenchmarkDocument/text_edit_gc_100 1284948 ns/op 870939 B/op 16752 allocs/op 1288558 ns/op 871001 B/op 16752 allocs/op 1.00
BenchmarkDocument/text_edit_gc_100 - ns/op 1284948 ns/op 1288558 ns/op 1.00
BenchmarkDocument/text_edit_gc_100 - B/op 870939 B/op 871001 B/op 1.00
BenchmarkDocument/text_edit_gc_100 - allocs/op 16752 allocs/op 16752 allocs/op 1
BenchmarkDocument/text_edit_gc_1000 50014439 ns/op 50536032 B/op 181716 allocs/op 50792997 ns/op 50537048 B/op 181723 allocs/op 0.98
BenchmarkDocument/text_edit_gc_1000 - ns/op 50014439 ns/op 50792997 ns/op 0.98
BenchmarkDocument/text_edit_gc_1000 - B/op 50536032 B/op 50537048 B/op 1.00
BenchmarkDocument/text_edit_gc_1000 - allocs/op 181716 allocs/op 181723 allocs/op 1.00
BenchmarkDocument/text_split_gc_100 1894438 ns/op 1528850 B/op 15605 allocs/op 1893590 ns/op 1528814 B/op 15605 allocs/op 1.00
BenchmarkDocument/text_split_gc_100 - ns/op 1894438 ns/op 1893590 ns/op 1.00
BenchmarkDocument/text_split_gc_100 - B/op 1528850 B/op 1528814 B/op 1.00
BenchmarkDocument/text_split_gc_100 - allocs/op 15605 allocs/op 15605 allocs/op 1
BenchmarkDocument/text_split_gc_1000 114447614 ns/op 135075785 B/op 182206 allocs/op 117031900 ns/op 135078265 B/op 182205 allocs/op 0.98
BenchmarkDocument/text_split_gc_1000 - ns/op 114447614 ns/op 117031900 ns/op 0.98
BenchmarkDocument/text_split_gc_1000 - B/op 135075785 B/op 135078265 B/op 1.00
BenchmarkDocument/text_split_gc_1000 - allocs/op 182206 allocs/op 182205 allocs/op 1.00
BenchmarkDocument/text_delete_all_10000 15473607 ns/op 10181726 B/op 40672 allocs/op 15387765 ns/op 10182724 B/op 40675 allocs/op 1.01
BenchmarkDocument/text_delete_all_10000 - ns/op 15473607 ns/op 15387765 ns/op 1.01
BenchmarkDocument/text_delete_all_10000 - B/op 10181726 B/op 10182724 B/op 1.00
BenchmarkDocument/text_delete_all_10000 - allocs/op 40672 allocs/op 40675 allocs/op 1.00
BenchmarkDocument/text_delete_all_100000 286675797 ns/op 142683276 B/op 411706 allocs/op 278753107 ns/op 142703176 B/op 411790 allocs/op 1.03
BenchmarkDocument/text_delete_all_100000 - ns/op 286675797 ns/op 278753107 ns/op 1.03
BenchmarkDocument/text_delete_all_100000 - B/op 142683276 B/op 142703176 B/op 1.00
BenchmarkDocument/text_delete_all_100000 - allocs/op 411706 allocs/op 411790 allocs/op 1.00
BenchmarkDocument/text_100 225071 ns/op 120036 B/op 5081 allocs/op 215751 ns/op 120037 B/op 5081 allocs/op 1.04
BenchmarkDocument/text_100 - ns/op 225071 ns/op 215751 ns/op 1.04
BenchmarkDocument/text_100 - B/op 120036 B/op 120037 B/op 1.00
BenchmarkDocument/text_100 - allocs/op 5081 allocs/op 5081 allocs/op 1
BenchmarkDocument/text_1000 2461410 ns/op 1169023 B/op 50085 allocs/op 2334394 ns/op 1169022 B/op 50085 allocs/op 1.05
BenchmarkDocument/text_1000 - ns/op 2461410 ns/op 2334394 ns/op 1.05
BenchmarkDocument/text_1000 - B/op 1169023 B/op 1169022 B/op 1.00
BenchmarkDocument/text_1000 - allocs/op 50085 allocs/op 50085 allocs/op 1
BenchmarkDocument/array_1000 1285756 ns/op 1091364 B/op 11831 allocs/op 1204103 ns/op 1091322 B/op 11831 allocs/op 1.07
BenchmarkDocument/array_1000 - ns/op 1285756 ns/op 1204103 ns/op 1.07
BenchmarkDocument/array_1000 - B/op 1091364 B/op 1091322 B/op 1.00
BenchmarkDocument/array_1000 - allocs/op 11831 allocs/op 11831 allocs/op 1
BenchmarkDocument/array_10000 13430853 ns/op 9800199 B/op 120297 allocs/op 13243491 ns/op 9799718 B/op 120295 allocs/op 1.01
BenchmarkDocument/array_10000 - ns/op 13430853 ns/op 13243491 ns/op 1.01
BenchmarkDocument/array_10000 - B/op 9800199 B/op 9799718 B/op 1.00
BenchmarkDocument/array_10000 - allocs/op 120297 allocs/op 120295 allocs/op 1.00
BenchmarkDocument/array_gc_100 155774 ns/op 132718 B/op 1260 allocs/op 143766 ns/op 132713 B/op 1260 allocs/op 1.08
BenchmarkDocument/array_gc_100 - ns/op 155774 ns/op 143766 ns/op 1.08
BenchmarkDocument/array_gc_100 - B/op 132718 B/op 132713 B/op 1.00
BenchmarkDocument/array_gc_100 - allocs/op 1260 allocs/op 1260 allocs/op 1
BenchmarkDocument/array_gc_1000 1453355 ns/op 1159143 B/op 12876 allocs/op 1375958 ns/op 1159179 B/op 12877 allocs/op 1.06
BenchmarkDocument/array_gc_1000 - ns/op 1453355 ns/op 1375958 ns/op 1.06
BenchmarkDocument/array_gc_1000 - B/op 1159143 B/op 1159179 B/op 1.00
BenchmarkDocument/array_gc_1000 - allocs/op 12876 allocs/op 12877 allocs/op 1.00
BenchmarkDocument/counter_1000 211507 ns/op 193080 B/op 5771 allocs/op 203008 ns/op 193079 B/op 5771 allocs/op 1.04
BenchmarkDocument/counter_1000 - ns/op 211507 ns/op 203008 ns/op 1.04
BenchmarkDocument/counter_1000 - B/op 193080 B/op 193079 B/op 1.00
BenchmarkDocument/counter_1000 - allocs/op 5771 allocs/op 5771 allocs/op 1
BenchmarkDocument/counter_10000 2221416 ns/op 2087997 B/op 59778 allocs/op 2172364 ns/op 2088011 B/op 59778 allocs/op 1.02
BenchmarkDocument/counter_10000 - ns/op 2221416 ns/op 2172364 ns/op 1.02
BenchmarkDocument/counter_10000 - B/op 2087997 B/op 2088011 B/op 1.00
BenchmarkDocument/counter_10000 - allocs/op 59778 allocs/op 59778 allocs/op 1
BenchmarkDocument/object_1000 1437556 ns/op 1427885 B/op 9848 allocs/op 1356540 ns/op 1428032 B/op 9849 allocs/op 1.06
BenchmarkDocument/object_1000 - ns/op 1437556 ns/op 1356540 ns/op 1.06
BenchmarkDocument/object_1000 - B/op 1427885 B/op 1428032 B/op 1.00
BenchmarkDocument/object_1000 - allocs/op 9848 allocs/op 9849 allocs/op 1.00
BenchmarkDocument/object_10000 15032425 ns/op 12167992 B/op 100567 allocs/op 14884932 ns/op 12167051 B/op 100564 allocs/op 1.01
BenchmarkDocument/object_10000 - ns/op 15032425 ns/op 14884932 ns/op 1.01
BenchmarkDocument/object_10000 - B/op 12167992 B/op 12167051 B/op 1.00
BenchmarkDocument/object_10000 - allocs/op 100567 allocs/op 100564 allocs/op 1.00
BenchmarkDocument/tree_100 1067738 ns/op 943704 B/op 6101 allocs/op 1013088 ns/op 943700 B/op 6101 allocs/op 1.05
BenchmarkDocument/tree_100 - ns/op 1067738 ns/op 1013088 ns/op 1.05
BenchmarkDocument/tree_100 - B/op 943704 B/op 943700 B/op 1.00
BenchmarkDocument/tree_100 - allocs/op 6101 allocs/op 6101 allocs/op 1
BenchmarkDocument/tree_1000 78787639 ns/op 86460468 B/op 60115 allocs/op 71992426 ns/op 86460268 B/op 60114 allocs/op 1.09
BenchmarkDocument/tree_1000 - ns/op 78787639 ns/op 71992426 ns/op 1.09
BenchmarkDocument/tree_1000 - B/op 86460468 B/op 86460268 B/op 1.00
BenchmarkDocument/tree_1000 - allocs/op 60115 allocs/op 60114 allocs/op 1.00
BenchmarkDocument/tree_10000 9598013170 ns/op 8580669280 B/op 600224 allocs/op 9143472905 ns/op 8580655920 B/op 600237 allocs/op 1.05
BenchmarkDocument/tree_10000 - ns/op 9598013170 ns/op 9143472905 ns/op 1.05
BenchmarkDocument/tree_10000 - B/op 8580669280 B/op 8580655920 B/op 1.00
BenchmarkDocument/tree_10000 - allocs/op 600224 allocs/op 600237 allocs/op 1.00
BenchmarkDocument/tree_delete_all_1000 81529903 ns/op 87510831 B/op 75267 allocs/op 72347571 ns/op 87530017 B/op 75262 allocs/op 1.13
BenchmarkDocument/tree_delete_all_1000 - ns/op 81529903 ns/op 72347571 ns/op 1.13
BenchmarkDocument/tree_delete_all_1000 - B/op 87510831 B/op 87530017 B/op 1.00
BenchmarkDocument/tree_delete_all_1000 - allocs/op 75267 allocs/op 75262 allocs/op 1.00
BenchmarkDocument/tree_edit_gc_100 3969216 ns/op 4146771 B/op 15141 allocs/op 3688585 ns/op 4146651 B/op 15140 allocs/op 1.08
BenchmarkDocument/tree_edit_gc_100 - ns/op 3969216 ns/op 3688585 ns/op 1.08
BenchmarkDocument/tree_edit_gc_100 - B/op 4146771 B/op 4146651 B/op 1.00
BenchmarkDocument/tree_edit_gc_100 - allocs/op 15141 allocs/op 15140 allocs/op 1.00
BenchmarkDocument/tree_edit_gc_1000 324722614 ns/op 383744200 B/op 154851 allocs/op 292242426 ns/op 383745892 B/op 154846 allocs/op 1.11
BenchmarkDocument/tree_edit_gc_1000 - ns/op 324722614 ns/op 292242426 ns/op 1.11
BenchmarkDocument/tree_edit_gc_1000 - B/op 383744200 B/op 383745892 B/op 1.00
BenchmarkDocument/tree_edit_gc_1000 - allocs/op 154851 allocs/op 154846 allocs/op 1.00
BenchmarkDocument/tree_split_gc_100 2668683 ns/op 2412500 B/op 11125 allocs/op 2473154 ns/op 2412517 B/op 11125 allocs/op 1.08
BenchmarkDocument/tree_split_gc_100 - ns/op 2668683 ns/op 2473154 ns/op 1.08
BenchmarkDocument/tree_split_gc_100 - B/op 2412500 B/op 2412517 B/op 1.00
BenchmarkDocument/tree_split_gc_100 - allocs/op 11125 allocs/op 11125 allocs/op 1
BenchmarkDocument/tree_split_gc_1000 196740186 ns/op 222253316 B/op 122005 allocs/op 176805670 ns/op 222249094 B/op 121984 allocs/op 1.11
BenchmarkDocument/tree_split_gc_1000 - ns/op 196740186 ns/op 176805670 ns/op 1.11
BenchmarkDocument/tree_split_gc_1000 - B/op 222253316 B/op 222249094 B/op 1.00
BenchmarkDocument/tree_split_gc_1000 - allocs/op 122005 allocs/op 121984 allocs/op 1.00
BenchmarkRPC/client_to_server 375054349 ns/op 16968181 B/op 175380 allocs/op 369454598 ns/op 17775253 B/op 175392 allocs/op 1.02
BenchmarkRPC/client_to_server - ns/op 375054349 ns/op 369454598 ns/op 1.02
BenchmarkRPC/client_to_server - B/op 16968181 B/op 17775253 B/op 0.95
BenchmarkRPC/client_to_server - allocs/op 175380 allocs/op 175392 allocs/op 1.00
BenchmarkRPC/client_to_client_via_server 629138452 ns/op 34641276 B/op 320771 allocs/op 623062984 ns/op 31794484 B/op 320964 allocs/op 1.01
BenchmarkRPC/client_to_client_via_server - ns/op 629138452 ns/op 623062984 ns/op 1.01
BenchmarkRPC/client_to_client_via_server - B/op 34641276 B/op 31794484 B/op 1.09
BenchmarkRPC/client_to_client_via_server - allocs/op 320771 allocs/op 320964 allocs/op 1.00
BenchmarkRPC/attach_large_document 1490855709 ns/op 1908548416 B/op 8864 allocs/op 1561940781 ns/op 1919901736 B/op 8849 allocs/op 0.95
BenchmarkRPC/attach_large_document - ns/op 1490855709 ns/op 1561940781 ns/op 0.95
BenchmarkRPC/attach_large_document - B/op 1908548416 B/op 1919901736 B/op 0.99
BenchmarkRPC/attach_large_document - allocs/op 8864 allocs/op 8849 allocs/op 1.00
BenchmarkRPC/adminCli_to_server 543571648 ns/op 35960348 B/op 289509 allocs/op 538036370 ns/op 35962604 B/op 289533 allocs/op 1.01
BenchmarkRPC/adminCli_to_server - ns/op 543571648 ns/op 538036370 ns/op 1.01
BenchmarkRPC/adminCli_to_server - B/op 35960348 B/op 35962604 B/op 1.00
BenchmarkRPC/adminCli_to_server - allocs/op 289509 allocs/op 289533 allocs/op 1.00
BenchmarkLocker 64.15 ns/op 16 B/op 1 allocs/op 63.21 ns/op 16 B/op 1 allocs/op 1.01
BenchmarkLocker - ns/op 64.15 ns/op 63.21 ns/op 1.01
BenchmarkLocker - B/op 16 B/op 16 B/op 1
BenchmarkLocker - allocs/op 1 allocs/op 1 allocs/op 1
BenchmarkLockerParallel 40.21 ns/op 0 B/op 0 allocs/op 38.91 ns/op 0 B/op 0 allocs/op 1.03
BenchmarkLockerParallel - ns/op 40.21 ns/op 38.91 ns/op 1.03
BenchmarkLockerParallel - B/op 0 B/op 0 B/op 1
BenchmarkLockerParallel - allocs/op 0 allocs/op 0 allocs/op 1
BenchmarkLockerMoreKeys 142.9 ns/op 15 B/op 0 allocs/op 140.7 ns/op 15 B/op 0 allocs/op 1.02
BenchmarkLockerMoreKeys - ns/op 142.9 ns/op 140.7 ns/op 1.02
BenchmarkLockerMoreKeys - B/op 15 B/op 15 B/op 1
BenchmarkLockerMoreKeys - allocs/op 0 allocs/op 0 allocs/op 1
BenchmarkChange/Push_10_Changes 3897972 ns/op 121528 B/op 1285 allocs/op 3882129 ns/op 121583 B/op 1284 allocs/op 1.00
BenchmarkChange/Push_10_Changes - ns/op 3897972 ns/op 3882129 ns/op 1.00
BenchmarkChange/Push_10_Changes - B/op 121528 B/op 121583 B/op 1.00
BenchmarkChange/Push_10_Changes - allocs/op 1285 allocs/op 1284 allocs/op 1.00
BenchmarkChange/Push_100_Changes 14483879 ns/op 571989 B/op 6654 allocs/op 14467650 ns/op 572603 B/op 6655 allocs/op 1.00
BenchmarkChange/Push_100_Changes - ns/op 14483879 ns/op 14467650 ns/op 1.00
BenchmarkChange/Push_100_Changes - B/op 571989 B/op 572603 B/op 1.00
BenchmarkChange/Push_100_Changes - allocs/op 6654 allocs/op 6655 allocs/op 1.00
BenchmarkChange/Push_1000_Changes 116295366 ns/op 5335016 B/op 63150 allocs/op 116074912 ns/op 5226132 B/op 63149 allocs/op 1.00
BenchmarkChange/Push_1000_Changes - ns/op 116295366 ns/op 116074912 ns/op 1.00
BenchmarkChange/Push_1000_Changes - B/op 5335016 B/op 5226132 B/op 1.02
BenchmarkChange/Push_1000_Changes - allocs/op 63150 allocs/op 63149 allocs/op 1.00
BenchmarkChange/Pull_10_Changes 2916445 ns/op 101009 B/op 1004 allocs/op 2886266 ns/op 101120 B/op 1004 allocs/op 1.01
BenchmarkChange/Pull_10_Changes - ns/op 2916445 ns/op 2886266 ns/op 1.01
BenchmarkChange/Pull_10_Changes - B/op 101009 B/op 101120 B/op 1.00
BenchmarkChange/Pull_10_Changes - allocs/op 1004 allocs/op 1004 allocs/op 1
BenchmarkChange/Pull_100_Changes 4376138 ns/op 266761 B/op 3475 allocs/op 4317620 ns/op 267409 B/op 3475 allocs/op 1.01
BenchmarkChange/Pull_100_Changes - ns/op 4376138 ns/op 4317620 ns/op 1.01
BenchmarkChange/Pull_100_Changes - B/op 266761 B/op 267409 B/op 1.00
BenchmarkChange/Pull_100_Changes - allocs/op 3475 allocs/op 3475 allocs/op 1
BenchmarkChange/Pull_1000_Changes 8537930 ns/op 1493831 B/op 29861 allocs/op 8477009 ns/op 1491793 B/op 29863 allocs/op 1.01
BenchmarkChange/Pull_1000_Changes - ns/op 8537930 ns/op 8477009 ns/op 1.01
BenchmarkChange/Pull_1000_Changes - B/op 1493831 B/op 1491793 B/op 1.00
BenchmarkChange/Pull_1000_Changes - allocs/op 29861 allocs/op 29863 allocs/op 1.00
BenchmarkSnapshot/Push_3KB_snapshot 17042190 ns/op 715124 B/op 6655 allocs/op 16841670 ns/op 713835 B/op 6655 allocs/op 1.01
BenchmarkSnapshot/Push_3KB_snapshot - ns/op 17042190 ns/op 16841670 ns/op 1.01
BenchmarkSnapshot/Push_3KB_snapshot - B/op 715124 B/op 713835 B/op 1.00
BenchmarkSnapshot/Push_3KB_snapshot - allocs/op 6655 allocs/op 6655 allocs/op 1
BenchmarkSnapshot/Push_30KB_snapshot 119061102 ns/op 5693377 B/op 63155 allocs/op 119792109 ns/op 5676793 B/op 63159 allocs/op 0.99
BenchmarkSnapshot/Push_30KB_snapshot - ns/op 119061102 ns/op 119792109 ns/op 0.99
BenchmarkSnapshot/Push_30KB_snapshot - B/op 5693377 B/op 5676793 B/op 1.00
BenchmarkSnapshot/Push_30KB_snapshot - allocs/op 63155 allocs/op 63159 allocs/op 1.00
BenchmarkSnapshot/Pull_3KB_snapshot 6454283 ns/op 923638 B/op 15513 allocs/op 6487252 ns/op 924577 B/op 15512 allocs/op 0.99
BenchmarkSnapshot/Pull_3KB_snapshot - ns/op 6454283 ns/op 6487252 ns/op 0.99
BenchmarkSnapshot/Pull_3KB_snapshot - B/op 923638 B/op 924577 B/op 1.00
BenchmarkSnapshot/Pull_3KB_snapshot - allocs/op 15513 allocs/op 15512 allocs/op 1.00
BenchmarkSnapshot/Pull_30KB_snapshot 15016084 ns/op 7146226 B/op 149938 allocs/op 15198765 ns/op 7156141 B/op 150108 allocs/op 0.99
BenchmarkSnapshot/Pull_30KB_snapshot - ns/op 15016084 ns/op 15198765 ns/op 0.99
BenchmarkSnapshot/Pull_30KB_snapshot - B/op 7146226 B/op 7156141 B/op 1.00
BenchmarkSnapshot/Pull_30KB_snapshot - allocs/op 149938 allocs/op 150108 allocs/op 1.00
BenchmarkSync/memory_sync_10_test 8152 ns/op 1286 B/op 38 allocs/op 6873 ns/op 1286 B/op 38 allocs/op 1.19
BenchmarkSync/memory_sync_10_test - ns/op 8152 ns/op 6873 ns/op 1.19
BenchmarkSync/memory_sync_10_test - B/op 1286 B/op 1286 B/op 1
BenchmarkSync/memory_sync_10_test - allocs/op 38 allocs/op 38 allocs/op 1
BenchmarkSync/memory_sync_100_test 58604 ns/op 8976 B/op 294 allocs/op 50675 ns/op 8643 B/op 273 allocs/op 1.16
BenchmarkSync/memory_sync_100_test - ns/op 58604 ns/op 50675 ns/op 1.16
BenchmarkSync/memory_sync_100_test - B/op 8976 B/op 8643 B/op 1.04
BenchmarkSync/memory_sync_100_test - allocs/op 294 allocs/op 273 allocs/op 1.08
BenchmarkSync/memory_sync_1000_test 417403 ns/op 82611 B/op 2638 allocs/op 583495 ns/op 74149 B/op 2108 allocs/op 0.72
BenchmarkSync/memory_sync_1000_test - ns/op 417403 ns/op 583495 ns/op 0.72
BenchmarkSync/memory_sync_1000_test - B/op 82611 B/op 74149 B/op 1.11
BenchmarkSync/memory_sync_1000_test - allocs/op 2638 allocs/op 2108 allocs/op 1.25
BenchmarkSync/memory_sync_10000_test 6125207 ns/op 752087 B/op 21266 allocs/op 7157593 ns/op 737153 B/op 20259 allocs/op 0.86
BenchmarkSync/memory_sync_10000_test - ns/op 6125207 ns/op 7157593 ns/op 0.86
BenchmarkSync/memory_sync_10000_test - B/op 752087 B/op 737153 B/op 1.02
BenchmarkSync/memory_sync_10000_test - allocs/op 21266 allocs/op 20259 allocs/op 1.05
BenchmarkTextEditing 4951147310 ns/op 3901963728 B/op 18743580 allocs/op 5104557048 ns/op 3901989536 B/op 18743642 allocs/op 0.97
BenchmarkTextEditing - ns/op 4951147310 ns/op 5104557048 ns/op 0.97
BenchmarkTextEditing - B/op 3901963728 B/op 3901989536 B/op 1.00
BenchmarkTextEditing - allocs/op 18743580 allocs/op 18743642 allocs/op 1.00

This comment was automatically generated by workflow using github-action-benchmark.

coderabbitai[bot]
coderabbitai bot reviewed Jul 4, 2024
View reviewed changes
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 58d45c6 and 7031160.

Files selected for processing (4)
  • pkg/document/crdt/rga_tree_split.go (1 hunks)
  • pkg/document/crdt/text.go (2 hunks)
  • pkg/document/document_test.go (1 hunks)
  • test/integration/text_test.go (1 hunks)
Additional comments not posted (5)
pkg/document/crdt/text.go (2)

25-26: Imports look good!

The new imports for llrb and splay are necessary for the added methods.

372-380: New methods look good!

The methods TreeByIndex and TreeByID are correctly implemented and follow best practices.

However, ensure that these methods are used only for debugging purposes as intended.

Verification successful

Confirmed: Methods used only in test files

The methods TreeByIndex and TreeByID are used exclusively in test files (text_test.go and document_test.go), confirming they are used for debugging purposes only.

  • test/integration/text_test.go
  • pkg/document/document_test.go
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the usage of the new methods TreeByIndex and TreeByID.

# Test: Search for the method usage. Expect: Used only for debugging purposes.
rg --type go $'TreeByIndex\(\)|TreeByID\(\)'

Length of output: 838

test/integration/text_test.go (1)

265-266: New assertions look good!

The new assertions check the weight of text in documents d1 and d2 using the TreeByIndex method, ensuring the internal state is correct.

pkg/document/document_test.go (1)

528-551: New test case looks good!

The new test case purge node from index during GC test is correctly implemented and ensures that nodes are purged from the index during garbage collection.

pkg/document/crdt/rga_tree_split.go (1)

631-632: Updates to Purge method look good!

The updates to the Purge method ensure that nodes are correctly deleted from treeByIndex and treeByID.

@hackerwins hackerwins self-requested a review July 4, 2024 05:21
hackerwins
hackerwins approved these changes Jul 4, 2024
View reviewed changes
Copy link
Member

@hackerwins hackerwins left a comment

Choose a reason for hiding this comment

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

LGTM.

@hackerwins hackerwins merged commit 8ab096e into main Jul 4, 2024
4 checks passed
@hackerwins hackerwins deleted the removing-from-indexes branch July 4, 2024 05:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@coderabbitai coderabbitai[bot] coderabbitai[bot] left review comments

@github-actions github-actions[bot] github-actions[bot] left review comments

@hackerwins hackerwins hackerwins approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@raararaara @hackerwins