reimplements deletion, removal, and traversal components to support multiple merge and future extensibility. #1420
Conversation
WalkthroughThe changes refactor and extend the CRDT tree and tree index logic. In the CRDT tree, new methods are introduced for clearing children, marshaling nodes, traversing positions, and modularizing node removal and merging. In the tree index, sibling traversal and offset calculation are enhanced to handle removed nodes, and child-setting logic is refined for text nodes. A test assertion is updated to reflect changed serialization of deleted nodes. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Tree
participant TreeNode
Client->>Tree: Edit(fromParent, fromLeft, toParent, toLeft, ...)
Tree->>Tree: collectDeleteAndMerge(fromParent, fromLeft, toParent, toLeft)
loop For each node to remove
Tree->>Tree: RemoveNode(node, editedAt, versionVector)
end
loop For each node to merge
Tree->>Tree: MergeNode(node, editedAt, versionVector)
end
Tree->>Client: Return edit result
sequenceDiagram
participant User
participant Node
User->>Node: NextSiblingExtended()
Node->>Node: Traverse siblings (including removed)
Node-->>User: Return next sibling or nil
Estimated code review effort🎯 4 (Complex) | ⏱️ ~40 minutes Possibly related PRs
Suggested reviewers
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (2.2.2)Error: can't load config: unsupported version of the configuration: "" See https://golangci-lint.run/product/migration-guide for migration instructions Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🔇 Additional comments (9)
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
pkg/document/crdt/tree.go (1)
1072-1111: Sophisticated merge implementation with proper child preservation.The logic correctly moves children from deletable siblings before marking them as removed, preserving document structure during concurrent edits.
Consider adding a comment explaining the merge strategy for future maintainers.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
pkg/document/crdt/tree.go(6 hunks)pkg/index/tree.go(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
pkg/document/crdt/tree.go (4)
pkg/resource/resource.go (1)
DataSize(31-34)pkg/document/crdt/gc.go (1)
GCPair(26-29)pkg/document/time/ticket.go (2)
Ticket(61-71)MaxLamport(31-31)pkg/document/time/version_vector.go (1)
VersionVector(33-33)
🔇 Additional comments (8)
pkg/index/tree.go (2)
320-323: LGTM! Sensible enhancement for clearing operations.The modification to allow
SetChildren(nil)on text nodes is appropriate, as it enables cleanup operations while still preventing invalid child assignment.
470-490: LGTM! Well-implemented optional parameter.The modification correctly adds support for including removed nodes in offset calculation while maintaining backward compatibility through the variadic parameter pattern.
pkg/document/crdt/tree.go (6)
242-245: LGTM! Clean abstraction for clearing children.The method provides a clear, semantic wrapper for the clearing operation.
602-608: LGTM! Useful for debugging with node identification.The method adds node ID information to the marshaled output, which is helpful for debugging tree operations.
867-899: Excellent refactoring for improved clarity and maintainability.The separation of concerns into
collectDeleteAndMerge,RemoveNode, andMergeNodemakes the edit logic much more understandable and maintainable. The control flow is now linear and easy to follow.
1014-1049: Well-designed algorithm for node classification.The stack-based approach elegantly distinguishes between deletable nodes (with both start/end tokens in range) and mergeable nodes (with only end token in range). The reversal of merges ensures correct processing order.
1051-1070: LGTM! Proper concurrency control for node removal.The method correctly handles version vector-based concurrency control and returns the removed node for garbage collection.
1163-1219: Well-structured tree traversal implementation.The separation of traversal logic into
traverseInTreePosandnextTreePosprovides clear abstractions. The case-by-case handling innextTreePoscorrectly covers all tree navigation scenarios.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
pkg/document/crdt/tree.go (3)
154-155: LGTM! Consider improving parameter naming for Go idioms.The variadic parameter approach maintains backward compatibility while adding flexibility. The implementation correctly handles the optional parameter with safe bounds checking.
Consider renaming the parameter to follow Go naming conventions:
-func (n *TreeNode) Children(includeRemovedNode ...bool) []*TreeNode { - children := n.Index.Children(len(includeRemovedNode) > 0 && includeRemovedNode[0]) +func (n *TreeNode) Children(includeRemoved ...bool) []*TreeNode { + children := n.Index.Children(len(includeRemoved) > 0 && includeRemoved[0])
948-985: LGTM! Consider adding documentation for the stack-based logic.The implementation correctly distinguishes between removable and mergeable nodes using a stack-based approach. The logic aligns with CRDT semantics where text nodes are always removed, and element nodes are classified based on their traversal pattern.
Consider adding inline comments to explain the stack logic:
+ // Stack tracks nested element nodes to determine removal vs merge var stack []*TreeNodeID err := traverseInTreePos(fromParent, fromLeft, toParent, toLeft, func(parent, left *TreeNode) error { if left.IsText() { removes = append(removes, left) return nil } if parent == left { - // push + // Push: entering a new element node stack = append(stack, left.ID()) } else { - // pop + // Pop: check if this closes a nested element if len(stack) > 0 && stack[len(stack)-1].Equals(left.ID()) { stack = stack[:len(stack)-1] + // Both start and end are deleted - remove the node removes = append(removes, left) } else { + // Only end is deleted - merge with siblings merges = append(merges, left) } }
987-1006: LGTM! Consider refactoring to reduce code duplication.The method correctly handles both local and remote operations with proper permission checking. The version vector logic ensures proper conflict resolution for concurrent edits.
Consider refactoring to reduce duplication:
func (t *Tree) RemoveNode(node *TreeNode, editedAt *time.Ticket, versionVector time.VersionVector) (*TreeNode, error) { actorID := node.id.CreatedAt.ActorID() - if len(versionVector) == 0 { - if node.remove(editedAt) { - return node, nil - } - } - clientLamportAtChange, ok := versionVector.Get(actorID) - if !ok { - return nil, nil - } - - if node.canDelete(editedAt, clientLamportAtChange) { + var canRemove bool + if len(versionVector) == 0 { + canRemove = true + } else { + clientLamportAtChange, ok := versionVector.Get(actorID) + if !ok { + return nil, nil + } + canRemove = node.canDelete(editedAt, clientLamportAtChange) + } + + if canRemove { if node.remove(editedAt) { return node, nil } } return nil, nil }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
pkg/document/crdt/tree.go(7 hunks)pkg/index/tree.go(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- pkg/index/tree.go
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: load-test
- GitHub Check: complex-test
- GitHub Check: bench
- GitHub Check: build
🔇 Additional comments (7)
pkg/document/crdt/tree.go (7)
242-245: LGTM!Clean implementation that properly delegates to the underlying index operation while maintaining error handling.
602-608: LGTM!Clean implementation that adds useful debugging capability by prefixing the JSON representation with creation time information.
867-899: Excellent refactoring that improves code organization and clarity.The separation of collection logic (
collectDeleteAndMerge) from execution logic (RemoveNode/MergeNode) enhances maintainability and aligns with single responsibility principle. The distinct handling of removes vs merges makes the logic clearer.
1008-1047: LGTM! Solid implementation of the merge operation.The method correctly implements sibling merging with proper version vector handling. The logic safely transfers children before removing the source node, and the extended sibling traversal ensures all potential merge candidates are considered.
1099-1119: LGTM! Clean abstraction for tree position traversal.The function provides a well-structured way to traverse between tree positions using a callback pattern. The implementation correctly handles the termination condition and error propagation.
1121-1155: LGTM! Comprehensive tree position advancement logic.The function correctly handles all traversal cases with proper distinction between text and element nodes. The implementation supports both depth-first traversal (into children) and sibling traversal with appropriate fallback to parent levels.
689-689: LGTM!Minor formatting improvement that maintains code consistency.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
pkg/document/crdt/tree.go (2)
154-155: Consider improving parameter validation for the optional boolean.The variadic parameter pattern is correctly implemented, but you could make the intent clearer and more robust:
-func (n *TreeNode) Children(includeRemovedNode ...bool) []*TreeNode { - children := n.Index.Children(len(includeRemovedNode) > 0 && includeRemovedNode[0]) +func (n *TreeNode) Children(includeRemovedNode ...bool) []*TreeNode { + includeRemoved := len(includeRemovedNode) > 0 && includeRemovedNode[0] + children := n.Index.Children(includeRemoved)This makes the boolean logic more explicit and easier to understand.
602-608: Good reuse of existing marshal logic with helpful ID prefix.The implementation correctly reuses the existing
marshalfunction and adds useful debugging information with the node ID. Consider whether the method nameMarshalmight be confusing given the existingTree.Marshalmethod - perhapsMarshalWithIDorDebugMarshalwould be clearer.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
pkg/document/crdt/tree.go(7 hunks)pkg/document/crdt/tree_test.go(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- pkg/document/crdt/tree_test.go
🔇 Additional comments (6)
pkg/document/crdt/tree.go (6)
242-245: LGTM! Clean delegation to index layer.The method provides a clear abstraction for clearing children and properly handles error propagation from the underlying index operation.
689-689: LGTM! Necessary JSON formatting fix.The comma addition ensures proper JSON structure when attributes are present.
867-985: Excellent refactor that significantly improves code clarity and maintainability.The new implementation is much cleaner:
- Clear separation of concerns: Distinct handling of removes vs merges
- Better abstraction: Using
traverseInTreePoseliminates complex nested conditions- Elegant stack-based tracking: Push/pop logic clearly handles element boundaries
- Improved classification: Clear distinction between deletable nodes (both start/end tokens deleted) and mergeable nodes (only end token deleted)
The logic correctly identifies that when
parent == left, we're at an element start (push to stack), and when we encounter the same element again, it's deletable. Otherwise, it's mergeable.The
slices.Reverse(merges)ensures proper processing order for merge operations.
987-1006: LGTM! Good modularization of node removal logic.The extracted method properly handles both version vector scenarios:
- Empty version vector: Falls back to simple
remove()logic for local operations- With version vector: Uses
canDelete()for proper convergence handling in concurrent scenariosThe return pattern (removed node vs nil) is consistent and clear.
1008-1050: Good extraction of merge logic, but verify single-sibling limitation.The method properly handles version vectors and performs necessary type checking before merging. The use of
NextSiblingExtended(),ClearChildren(), andAppend()is clean and follows the new patterns.However, the method returns after processing the first mergeable sibling (Line 1043-1046). Is this intentional? In scenarios where multiple consecutive siblings could be merged, this might leave the tree in a partially merged state.
Please verify if this single-sibling processing is the intended behavior or if the method should continue merging all eligible consecutive siblings.
1102-1158: Excellent abstraction for tree position traversal.These functions provide a clean, reusable abstraction for the complex tree position traversal logic:
traverseInTreePos: Clean iteration interface with proper termination conditionnextTreePos: Correctly handles the four main cases:
- Parent == left with children: Move to leftmost child
- Parent == left without children: Move up to parent
- Parent != left with next sibling: Move to next sibling
- Parent != left at end: Move up to parent
The logic properly distinguishes between text and element nodes, and the use of
Children(true)ensures removed nodes remain traversable - crucial for the improved convergence handling.This abstraction eliminates the complex nested conditions that were previously scattered throughout the codebase.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1420 +/- ##
==========================================
+ Coverage 38.11% 38.17% +0.05%
==========================================
Files 184 184
Lines 31080 31202 +122
==========================================
+ Hits 11847 11910 +63
- Misses 18281 18331 +50
- Partials 952 961 +9 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Go Benchmark Analysis 📊
This is a comparison result between the previous(851db0e) and the current commit(4728958).
Significant Changes (≥20% difference)
| Benchmark suite | Previous | Current | Change |
|---|---|---|---|
| BenchmarkDeletionConcurrency/concurrent_tree_delete_range_1000/ (B/op) | 886.60 MB | 654.83 MB | 🟢 -26.14% |
| BenchmarkDeletionConcurrency/concurrent_tree_edit_delete_all_1000/ (B/op) | 719.93 MB | 531.05 MB | 🟢 -26.24% |
| BenchmarkDocument/tree_100/ (ns/op) | 756344.00 ns | 448487.00 ns | 🟢 -40.70% |
| BenchmarkDocument/tree_100/ (B/op) | 524.17 KB | 347.97 KB | 🟢 -33.61% |
| BenchmarkDocument/tree_100/ (allocs/op) | 4,722 allocs | 3,526 allocs | 🟢 -25.33% |
| BenchmarkDocument/tree_1000/ (ns/op) | 51.54 ms | 23.08 ms | 🟢 -55.21% |
| BenchmarkDocument/tree_1000/ (B/op) | 43.74 MB | 26.55 MB | 🟢 -39.30% |
| BenchmarkDocument/tree_1000/ (allocs/op) | 46,131 allocs | 34,135 allocs | 🟢 -26.00% |
| BenchmarkDocument/tree_10000/ (ns/op) | 6.69 s | 2.69 s | 🟢 -59.74% |
| BenchmarkDocument/tree_10000/ (allocs/op) | 460,161 allocs | 340,172 allocs | 🟢 -26.08% |
| BenchmarkDocument/tree_edit_gc_1000/ (ns/op) | 223.96 ms | 164.76 ms | 🟢 -26.43% |
| BenchmarkDocument/tree_edit_gc_1000/ (B/op) | 213.94 MB | 158.48 MB | 🟢 -25.93% |
| BenchmarkDocumentDeletion/single_tree_delete_all_1000/ (ns/op) | 55.10 ms | 24.65 ms | 🟢 -55.27% |
| BenchmarkDocumentDeletion/single_tree_delete_all_1000/ (B/op) | 44.66 MB | 27.44 MB | 🟢 -38.56% |
| BenchmarkDocumentDeletion/single_tree_delete_all_1000/ (allocs/op) | 58,201 allocs | 46,193 allocs | 🟢 -20.63% |
| BenchmarkDocumentDeletion/single_tree_delete_range_100/ (ns/op) | 1.00 ms | 686154.00 ns | 🟢 -31.47% |
| BenchmarkDocumentDeletion/single_tree_delete_range_100/ (B/op) | 708.40 KB | 516.27 KB | 🟢 -27.12% |
| BenchmarkDocumentDeletion/single_tree_delete_range_100/ (allocs/op) | 6,392 allocs | 5,096 allocs | 🟢 -20.28% |
| BenchmarkDocumentDeletion/single_tree_delete_range_1000/ (ns/op) | 63.72 ms | 35.68 ms | 🟢 -44.01% |
| BenchmarkDocumentDeletion/single_tree_delete_range_1000/ (B/op) | 54.51 MB | 36.15 MB | 🟢 -33.68% |
| BenchmarkVersionVector/clients_1000/ (3_pushpull(ms)) | 8.00 ms | 5.00 ms | 🟢 -37.50% |
Key Observations 🔍
- The benchmark suite for
BenchmarkDeletionConcurrencyshows a significant improvement in memory usage forconcurrent_tree_delete_range_1000/,concurrent_tree_edit_delete_all_1000/, andconcurrent_tree_delete_range_100/. - In the
BenchmarkDocumentsuite, there are notable decreases in memory usage and allocations for various tree sizes, indicating potential optimization in memory management. - The benchmark suite for
BenchmarkDocumentDeletiondemonstrates notable reductions in operation time, memory usage, and allocations for deletion operations on trees and texts. - Overall, significant optimizations are evident in memory management, execution times, and allocation counts across various benchmark suites, highlighting potential efficiency improvements in the tested functionalities.
Clock Analysis
Lamport vs VersionVector
BenchmarkVersionVector Test
Lamport (v0.5.2)
| Metric | 10 clients | 100 clients | 1000 clients |
|---|---|---|---|
| Total Operation Time | 84.96 ms | 793.94 ms | 34.79 s |
| Memory Allocations | 35.11 MB | 219.92 MB | 4.91 GB |
| Number of Allocations (allocs/op) | 69,271 | 1,246,728 | 81,485,288 |
| ChangePack Size | 138.0 B | 137.0 B | 141.0 B |
| Snapshot Size | 379.0 B | 3.08 KB | 30.08 KB |
| Push-Pull Time | 2.0 ms | 1.5 ms | 4.0 ms |
| Attach Time | 4.5 ms | 11.0 ms | 31.0 ms |
| ChangePack After Detach | 138.0 B | 140.0 B | 141.0 B |
| Snapshot After Detach | 136.0 B | 137.0 B | 139.0 B |
| Push-Pull After Detach | 2.5 ms | 5.0 ms | 9.5 ms |
Version Vector (current)
| Metric | 10 clients | 100 clients | 1000 clients |
|---|---|---|---|
| Total Operation Time | 125.67 ms | 1.04 s | 11.92 s |
| Memory Allocations | 8.66 MB | 78.02 MB | 1.50 GB |
| Number of Allocations (allocs/op) | 65,776 | 648,354 | 18,347,284 |
| ChangePack Size | 185.00 B | 185.00 B | 186.00 B |
| Snapshot Size | 399.00 B | 3.10 KB | 30.10 KB |
| Push-Pull Time | 4.00 ms | 4.00 ms | 5.00 ms |
| Attach Time | 5.00 ms | 6.00 ms | 19.00 ms |
| ChangePack After Detach | 230.00 B | 231.00 B | 231.00 B |
| Snapshot After Detach | 156.00 B | 156.00 B | 156.00 B |
| Push-Pull After Detach | 4.00 ms | 5.00 ms | 5.00 ms |
BenchmarkSyncConcurrency Test
| Metric | Clients | Lamport (v0.5.2) | Version Vector (current) |
|---|---|---|---|
| Total Operation Time | 1-100-10-10 | 7.48 s | 8.04 s |
| 100-100-10-10 | 9.62 s | 9.27 s | |
| 300_100-10-10 | 14.77 s | 11.36 s | |
| Memory Allocations | 1-100-10-10 | 1.15 GB | 4.32 GB |
| 100-100-10-10 | 1.45 GB | 4.48 GB | |
| 300_100-10-10 | 2.19 GB | 4.67 GB | |
| Number of Allocations (allocs/op) | 1-100-10-10 | 17,107,766 | 106,529,729 |
| 100-100-10-10 | 20,084,480 | 108,019,859 | |
| 300_100-10-10 | 30,359,215 | 110,618,614 |
Summary
- The Version Vector clock system shows slightly higher operation times compared to Lamport for all levels of clients (10, 100, 1000). However, it also demonstrates higher memory allocations and number of allocations per operation as the number of clients increases, indicating potentially higher resource consumption compared to Lamport.
- In the BenchmarkSyncConcurrency tests, the Version Vector clock system generally performs slower than Lamport in terms of total operation times for different client configurations. Additionally, it exhibits higher memory allocations and number of allocations per operation across all client scenarios, suggesting a heavier resource utilization compared to Lamport.
Detailed Test Results
BenchmarkDeletionConcurrency
| Benchmark suite | Previous | Current | Change |
|---|---|---|---|
| concurrent_text_delete_range_100/ (ns/op) | 741.18 ms | 743.23 ms | 🔴 +0.28% |
| concurrent_text_delete_range_100/ (B/op) | 61.89 MB | 63.13 MB | 🔴 +2.00% |
| concurrent_text_delete_range_100/ (allocs/op) | 668,408 allocs | 668,407 allocs | ⚪ 0% |
| concurrent_text_delete_range_1000/ (ns/op) | 6.10 s | 6.07 s | 🟢 -0.52% |
| concurrent_text_delete_range_1000/ (B/op) | 365.60 MB | 364.72 MB | 🟢 -0.24% |
| concurrent_text_delete_range_1000/ (allocs/op) | 6,272,755 allocs | 6,272,358 allocs | ⚪ 0% |
| concurrent_tree_delete_range_100/ (ns/op) | 766.51 ms | 781.05 ms | 🔴 +1.90% |
| concurrent_tree_delete_range_100/ (B/op) | 68.26 MB | 65.14 MB | 🟢 -4.57% |
| concurrent_tree_delete_range_100/ (allocs/op) | 757,965 allocs | 743,589 allocs | 🟢 -1.90% |
| concurrent_tree_delete_range_1000/ (ns/op) | 6.82 s | 6.32 s | 🟢 -7.22% |
| concurrent_tree_delete_range_1000/ (B/op) | 886.60 MB | 654.83 MB | 🟢 -26.14% |
| concurrent_tree_delete_range_1000/ (allocs/op) | 6,653,578 allocs | 6,505,394 allocs | 🟢 -2.23% |
| concurrent_text_edit_delete_all_100/ (ns/op) | 648.64 ms | 643.90 ms | 🟢 -0.73% |
| concurrent_text_edit_delete_all_100/ (B/op) | 53.60 MB | 54.14 MB | 🔴 +1.01% |
| concurrent_text_edit_delete_all_100/ (allocs/op) | 566,604 allocs | 566,569 allocs | ⚪ 0% |
| concurrent_text_edit_delete_all_1000/ (ns/op) | 5.08 s | 5.09 s | 🔴 +0.14% |
| concurrent_text_edit_delete_all_1000/ (B/op) | 353.85 MB | 339.72 MB | 🟢 -3.99% |
| concurrent_text_edit_delete_all_1000/ (allocs/op) | 5,221,733 allocs | 5,214,687 allocs | 🟢 -0.13% |
| concurrent_tree_edit_delete_all_100/ (ns/op) | 656.05 ms | 655.65 ms | 🟢 -0.06% |
| concurrent_tree_edit_delete_all_100/ (B/op) | 58.68 MB | 58.48 MB | 🟢 -0.34% |
| concurrent_tree_edit_delete_all_100/ (allocs/op) | 613,267 allocs | 601,313 allocs | 🟢 -1.95% |
| concurrent_tree_edit_delete_all_1000/ (ns/op) | 5.69 s | 5.34 s | 🟢 -6.17% |
| concurrent_tree_edit_delete_all_1000/ (B/op) | 719.93 MB | 531.05 MB | 🟢 -26.24% |
| concurrent_tree_edit_delete_all_1000/ (allocs/op) | 5,638,931 allocs | 5,515,763 allocs | 🟢 -2.18% |
BenchmarkDocument
| Benchmark suite | Previous | Current | Change |
|---|---|---|---|
| constructor_test/ (ns/op) | 1621.00 ns | 1631.00 ns | 🔴 +0.62% |
| constructor_test/ (B/op) | 1.59 KB | 1.59 KB | ⚪ 0% |
| constructor_test/ (allocs/op) | 27 allocs | 27 allocs | ⚪ 0% |
| status_test/ (ns/op) | 1205.00 ns | 1215.00 ns | 🔴 +0.83% |
| status_test/ (B/op) | 1.56 KB | 1.56 KB | ⚪ 0% |
| status_test/ (allocs/op) | 25 allocs | 25 allocs | ⚪ 0% |
| equals_test/ (ns/op) | 8459.00 ns | 8525.00 ns | 🔴 +0.78% |
| equals_test/ (B/op) | 8.33 KB | 8.33 KB | ⚪ 0% |
| equals_test/ (allocs/op) | 139 allocs | 139 allocs | ⚪ 0% |
| nested_update_test/ (ns/op) | 18182.00 ns | 18038.00 ns | 🟢 -0.79% |
| nested_update_test/ (B/op) | 12.86 KB | 12.86 KB | ⚪ 0% |
| nested_update_test/ (allocs/op) | 272 allocs | 272 allocs | ⚪ 0% |
| delete_test/ (ns/op) | 24038.00 ns | 24197.00 ns | 🔴 +0.66% |
| delete_test/ (B/op) | 16.37 KB | 16.37 KB | ⚪ 0% |
| delete_test/ (allocs/op) | 352 allocs | 352 allocs | ⚪ 0% |
| object_test/ (ns/op) | 9111.00 ns | 9120.00 ns | 🔴 +0.10% |
| object_test/ (B/op) | 7.42 KB | 7.42 KB | ⚪ 0% |
| object_test/ (allocs/op) | 124 allocs | 124 allocs | ⚪ 0% |
| array_test/ (ns/op) | 29266.00 ns | 29675.00 ns | 🔴 +1.40% |
| array_test/ (B/op) | 12.64 KB | 12.64 KB | ⚪ 0% |
| array_test/ (allocs/op) | 283 allocs | 283 allocs | ⚪ 0% |
| text_test/ (ns/op) | 33661.00 ns | 33926.00 ns | 🔴 +0.79% |
| text_test/ (B/op) | 15.31 KB | 15.31 KB | ⚪ 0% |
| text_test/ (allocs/op) | 503 allocs | 503 allocs | ⚪ 0% |
| text_composition_test/ (ns/op) | 32412.00 ns | 32617.00 ns | 🔴 +0.63% |
| text_composition_test/ (B/op) | 16.94 KB | 16.94 KB | ⚪ 0% |
| text_composition_test/ (allocs/op) | 493 allocs | 493 allocs | ⚪ 0% |
| rich_text_test/ (ns/op) | 87555.00 ns | 87928.00 ns | 🔴 +0.43% |
| rich_text_test/ (B/op) | 38.48 KB | 38.48 KB | ⚪ 0% |
| rich_text_test/ (allocs/op) | 1,185 allocs | 1,185 allocs | ⚪ 0% |
| counter_test/ (ns/op) | 18554.00 ns | 18542.00 ns | 🟢 -0.06% |
| counter_test/ (B/op) | 12.31 KB | 12.32 KB | 🔴 +0.02% |
| counter_test/ (allocs/op) | 254 allocs | 254 allocs | ⚪ 0% |
| text_edit_gc_100/ (ns/op) | 1.45 ms | 1.45 ms | 🔴 +0.18% |
| text_edit_gc_100/ (B/op) | 808.71 KB | 808.45 KB | 🟢 -0.03% |
| text_edit_gc_100/ (allocs/op) | 16,885 allocs | 16,885 allocs | ⚪ 0% |
| text_edit_gc_1000/ (ns/op) | 55.13 ms | 56.33 ms | 🔴 +2.17% |
| text_edit_gc_1000/ (B/op) | 46.27 MB | 46.27 MB | ⚪ 0% |
| text_edit_gc_1000/ (allocs/op) | 181,594 allocs | 181,589 allocs | ⚪ 0% |
| text_split_gc_100/ (ns/op) | 2.16 ms | 2.18 ms | 🔴 +0.72% |
| text_split_gc_100/ (B/op) | 1.53 MB | 1.53 MB | ⚪ 0% |
| text_split_gc_100/ (allocs/op) | 15,553 allocs | 15,554 allocs | ⚪ 0% |
| text_split_gc_1000/ (ns/op) | 133.33 ms | 135.12 ms | 🔴 +1.35% |
| text_split_gc_1000/ (B/op) | 137.28 MB | 137.28 MB | ⚪ 0% |
| text_split_gc_1000/ (allocs/op) | 180,988 allocs | 180,992 allocs | ⚪ 0% |
| text_100/ (ns/op) | 243870.00 ns | 243101.00 ns | 🟢 -0.32% |
| text_100/ (B/op) | 113.26 KB | 113.26 KB | ⚪ 0% |
| text_100/ (allocs/op) | 4,985 allocs | 4,985 allocs | ⚪ 0% |
| text_1000/ (ns/op) | 2.69 ms | 2.65 ms | 🟢 -1.56% |
| text_1000/ (B/op) | 1.08 MB | 1.08 MB | ⚪ 0% |
| text_1000/ (allocs/op) | 49,088 allocs | 49,088 allocs | ⚪ 0% |
| array_1000/ (ns/op) | 1.35 ms | 1.36 ms | 🔴 +0.56% |
| array_1000/ (B/op) | 1.13 MB | 1.13 MB | ⚪ 0% |
| array_1000/ (allocs/op) | 13,883 allocs | 13,883 allocs | ⚪ 0% |
| array_10000/ (ns/op) | 15.07 ms | 14.97 ms | 🟢 -0.66% |
| array_10000/ (B/op) | 10.29 MB | 10.29 MB | 🟢 -0.02% |
| array_10000/ (allocs/op) | 140,742 allocs | 140,736 allocs | ⚪ 0% |
| array_gc_100/ (ns/op) | 143032.00 ns | 146019.00 ns | 🔴 +2.09% |
| array_gc_100/ (B/op) | 104.27 KB | 104.28 KB | ⚪ 0% |
| array_gc_100/ (allocs/op) | 1,469 allocs | 1,469 allocs | ⚪ 0% |
| array_gc_1000/ (ns/op) | 1.53 ms | 1.55 ms | 🔴 +1.42% |
| array_gc_1000/ (B/op) | 1.18 MB | 1.18 MB | ⚪ 0% |
| array_gc_1000/ (allocs/op) | 14,929 allocs | 14,929 allocs | ⚪ 0% |
| counter_1000/ (ns/op) | 210134.00 ns | 208529.00 ns | 🟢 -0.76% |
| counter_1000/ (B/op) | 194.39 KB | 194.39 KB | ⚪ 0% |
| counter_1000/ (allocs/op) | 5,773 allocs | 5,773 allocs | ⚪ 0% |
| counter_10000/ (ns/op) | 2.25 ms | 2.25 ms | 🔴 +0.30% |
| counter_10000/ (B/op) | 2.23 MB | 2.23 MB | ⚪ 0% |
| counter_10000/ (allocs/op) | 59,780 allocs | 59,780 allocs | ⚪ 0% |
| object_1000/ (ns/op) | 1.56 ms | 1.58 ms | 🔴 +1.28% |
| object_1000/ (B/op) | 1.48 MB | 1.48 MB | ⚪ 0% |
| object_1000/ (allocs/op) | 11,927 allocs | 11,927 allocs | ⚪ 0% |
| object_10000/ (ns/op) | 17.06 ms | 17.27 ms | 🔴 +1.22% |
| object_10000/ (B/op) | 12.75 MB | 12.75 MB | 🟢 -0.02% |
| object_10000/ (allocs/op) | 121,239 allocs | 121,232 allocs | ⚪ 0% |
| tree_100/ (ns/op) | 756344.00 ns | 448487.00 ns | 🟢 -40.70% |
| tree_100/ (B/op) | 524.17 KB | 347.97 KB | 🟢 -33.61% |
| tree_100/ (allocs/op) | 4,722 allocs | 3,526 allocs | 🟢 -25.33% |
| tree_1000/ (ns/op) | 51.54 ms | 23.08 ms | 🟢 -55.21% |
| tree_1000/ (B/op) | 43.74 MB | 26.55 MB | 🟢 -39.30% |
| tree_1000/ (allocs/op) | 46,131 allocs | 34,135 allocs | 🟢 -26.00% |
| tree_10000/ (ns/op) | 6.69 s | 2.69 s | 🟢 -59.74% |
| tree_10000/ (B/op) | 4.30 GB | 2.58 GB | 🟢 -39.92% |
| tree_10000/ (allocs/op) | 460,161 allocs | 340,172 allocs | 🟢 -26.08% |
| tree_edit_gc_100/ (ns/op) | 2.76 ms | 2.23 ms | 🟢 -19.24% |
| tree_edit_gc_100/ (B/op) | 2.40 MB | 1.86 MB | 🟢 -22.21% |
| tree_edit_gc_100/ (allocs/op) | 11,668 allocs | 10,218 allocs | 🟢 -12.43% |
| tree_edit_gc_1000/ (ns/op) | 223.96 ms | 164.76 ms | 🟢 -26.43% |
| tree_edit_gc_1000/ (B/op) | 213.94 MB | 158.48 MB | 🟢 -25.93% |
| tree_edit_gc_1000/ (allocs/op) | 116,145 allocs | 107,826 allocs | 🟢 -7.16% |
| tree_split_gc_100/ (ns/op) | 2.13 ms | 2.07 ms | 🟢 -3.01% |
| tree_split_gc_100/ (B/op) | 1.61 MB | 1.41 MB | 🟢 -12.43% |
| tree_split_gc_100/ (allocs/op) | 9,606 allocs | 9,344 allocs | 🟢 -2.73% |
| tree_split_gc_1000/ (ns/op) | 157.23 ms | 148.77 ms | 🟢 -5.38% |
| tree_split_gc_1000/ (B/op) | 149.85 MB | 128.85 MB | 🟢 -14.01% |
| tree_split_gc_1000/ (allocs/op) | 107,760 allocs | 111,424 allocs | 🔴 +3.40% |
BenchmarkDocumentDeletion
| Benchmark suite | Previous | Current | Change |
|---|---|---|---|
| single_text_delete_all_10000/ (ns/op) | 18.55 ms | 18.79 ms | 🔴 +1.32% |
| single_text_delete_all_10000/ (B/op) | 10.51 MB | 10.51 MB | ⚪ 0% |
| single_text_delete_all_10000/ (allocs/op) | 76,134 allocs | 76,134 allocs | ⚪ 0% |
| single_text_delete_all_100000/ (ns/op) | 272.56 ms | 308.09 ms | 🔴 +13.04% |
| single_text_delete_all_100000/ (B/op) | 104.76 MB | 104.77 MB | ⚪ 0% |
| single_text_delete_all_100000/ (allocs/op) | 766,147 allocs | 766,103 allocs | ⚪ 0% |
| single_tree_delete_all_1000/ (ns/op) | 55.10 ms | 24.65 ms | 🟢 -55.27% |
| single_tree_delete_all_1000/ (B/op) | 44.66 MB | 27.44 MB | 🟢 -38.56% |
| single_tree_delete_all_1000/ (allocs/op) | 58,201 allocs | 46,193 allocs | 🟢 -20.63% |
| single_text_delete_range_100/ (ns/op) | 476158.00 ns | 470896.00 ns | 🟢 -1.11% |
| single_text_delete_range_100/ (B/op) | 243.25 KB | 243.28 KB | 🔴 +0.01% |
| single_text_delete_range_100/ (allocs/op) | 7,074 allocs | 7,074 allocs | ⚪ 0% |
| single_text_delete_range_1000/ (ns/op) | 8.99 ms | 9.04 ms | 🔴 +0.55% |
| single_text_delete_range_1000/ (B/op) | 6.37 MB | 6.37 MB | ⚪ 0% |
| single_text_delete_range_1000/ (allocs/op) | 73,369 allocs | 73,369 allocs | ⚪ 0% |
| single_tree_delete_range_100/ (ns/op) | 1.00 ms | 686154.00 ns | 🟢 -31.47% |
| single_tree_delete_range_100/ (B/op) | 708.40 KB | 516.27 KB | 🟢 -27.12% |
| single_tree_delete_range_100/ (allocs/op) | 6,392 allocs | 5,096 allocs | 🟢 -20.28% |
| single_tree_delete_range_1000/ (ns/op) | 63.72 ms | 35.68 ms | 🟢 -44.01% |
| single_tree_delete_range_1000/ (B/op) | 54.51 MB | 36.15 MB | 🟢 -33.68% |
| single_tree_delete_range_1000/ (allocs/op) | 63,433 allocs | 51,015 allocs | 🟢 -19.58% |
BenchmarkRPC
| Benchmark suite | Previous | Current | Change |
|---|---|---|---|
| client_to_server/ (ns/op) | 257.87 ms | 258.94 ms | 🔴 +0.41% |
| client_to_server/ (B/op) | 15.90 MB | 15.26 MB | 🟢 -4.06% |
| client_to_server/ (allocs/op) | 183,122 allocs | 183,145 allocs | 🔴 +0.01% |
| client_to_client_via_server/ (ns/op) | 193.56 ms | 190.66 ms | 🟢 -1.50% |
| client_to_client_via_server/ (B/op) | 22.94 MB | 24.46 MB | 🔴 +6.61% |
| client_to_client_via_server/ (allocs/op) | 293,666 allocs | 293,341 allocs | 🟢 -0.11% |
| attach_large_document/ (ns/op) | 1.39 s | 1.36 s | 🟢 -2.46% |
| attach_large_document/ (B/op) | 1.85 GB | 1.85 GB | ⚪ 0% |
| attach_large_document/ (allocs/op) | 12,136 allocs | 11,904 allocs | 🟢 -1.91% |
| adminCli_to_server/ (ns/op) | 584.66 ms | 587.30 ms | 🔴 +0.45% |
| adminCli_to_server/ (B/op) | 34.71 MB | 37.62 MB | 🔴 +8.38% |
| adminCli_to_server/ (allocs/op) | 401,717 allocs | 401,766 allocs | 🔴 +0.01% |
BenchmarkLocker
| Benchmark suite | Previous | Current | Change |
|---|---|---|---|
| (ns/op) | 84.20 ns | 85.36 ns | 🔴 +1.38% |
| (B/op) | 32.00 B | 32.00 B | ⚪ 0% |
| (allocs/op) | 1 allocs | 1 allocs | ⚪ 0% |
BenchmarkLockerParallel
| Benchmark suite | Previous | Current | Change |
|---|---|---|---|
| (ns/op) | 48.64 ns | 47.25 ns | 🟢 -2.86% |
| (B/op) | 0.00 B | 0.00 B | ⚪ 0% |
| (allocs/op) | 0 allocs | 0 allocs | ⚪ 0% |
BenchmarkLockerMoreKeys
| Benchmark suite | Previous | Current | Change |
|---|---|---|---|
| (ns/op) | 190.50 ns | 191.40 ns | 🔴 +0.47% |
| (B/op) | 30.00 B | 30.00 B | ⚪ 0% |
| (allocs/op) | 0 allocs | 0 allocs | ⚪ 0% |
BenchmarkRWLocker
| Benchmark suite | Previous | Current | Change |
|---|---|---|---|
| RWLock_rate_2/ (ns/op) | 53.88 ns | 50.94 ns | 🟢 -5.46% |
| RWLock_rate_2/ (B/op) | 0.00 B | 0.00 B | ⚪ 0% |
| RWLock_rate_2/ (allocs/op) | 0 allocs | 0 allocs | ⚪ 0% |
| RWLock_rate_10/ (ns/op) | 48.94 ns | 45.61 ns | 🟢 -6.80% |
| RWLock_rate_10/ (B/op) | 0.00 B | 0.00 B | ⚪ 0% |
| RWLock_rate_10/ (allocs/op) | 0 allocs | 0 allocs | ⚪ 0% |
| RWLock_rate_100/ (ns/op) | 68.80 ns | 59.20 ns | 🟢 -13.95% |
| RWLock_rate_100/ (B/op) | 2.00 B | 1.00 B | 🟢 -50.00% |
| RWLock_rate_100/ (allocs/op) | 0 allocs | 0 allocs | ⚪ 0% |
| RWLock_rate_1000/ (ns/op) | 97.13 ns | 90.00 ns | 🟢 -7.34% |
| RWLock_rate_1000/ (B/op) | 8.00 B | 8.00 B | ⚪ 0% |
| RWLock_rate_1000/ (allocs/op) | 0 allocs | 0 allocs | ⚪ 0% |
BenchmarkPresenceConcurrency
| Benchmark suite | Previous | Current | Change |
|---|---|---|---|
| 0-100-10/ (ns/op) | 954.79 ms | 969.16 ms | 🔴 +1.50% |
| 0-100-10/ (B/op) | 410.61 MB | 412.10 MB | 🔴 +0.36% |
| 0-100-10/ (allocs/op) | 5,160,069 allocs | 5,158,786 allocs | 🟢 -0.02% |
| 100-100-10/ (ns/op) | 989.95 ms | 1.01 s | 🔴 +1.75% |
| 100-100-10/ (B/op) | 439.36 MB | 434.40 MB | 🟢 -1.13% |
| 100-100-10/ (allocs/op) | 5,646,485 allocs | 5,643,713 allocs | 🟢 -0.05% |
| 300-100-10/ (ns/op) | 1.05 s | 1.08 s | 🔴 +2.94% |
| 300-100-10/ (B/op) | 499.79 MB | 499.54 MB | 🟢 -0.05% |
| 300-100-10/ (allocs/op) | 6,619,744 allocs | 6,617,543 allocs | 🟢 -0.03% |
BenchmarkChange
| Benchmark suite | Previous | Current | Change |
|---|---|---|---|
| Push_10_Changes/ (ns/op) | 4.07 ms | 4.16 ms | 🔴 +2.35% |
| Push_10_Changes/ (B/op) | 204.86 KB | 204.70 KB | 🟢 -0.07% |
| Push_10_Changes/ (allocs/op) | 1,973 allocs | 1,974 allocs | 🔴 +0.05% |
| Push_100_Changes/ (ns/op) | 17.35 ms | 17.78 ms | 🔴 +2.48% |
| Push_100_Changes/ (B/op) | 1.33 MB | 1.33 MB | 🔴 +0.45% |
| Push_100_Changes/ (allocs/op) | 10,627 allocs | 10,629 allocs | 🔴 +0.02% |
| Push_1000_Changes/ (ns/op) | 149.22 ms | 151.16 ms | 🔴 +1.30% |
| Push_1000_Changes/ (B/op) | 13.37 MB | 13.43 MB | 🔴 +0.45% |
| Push_1000_Changes/ (allocs/op) | 99,919 allocs | 99,916 allocs | ⚪ 0% |
| Pull_10_Changes/ (ns/op) | 2.36 ms | 2.42 ms | 🔴 +2.44% |
| Pull_10_Changes/ (B/op) | 142.82 KB | 142.79 KB | 🟢 -0.02% |
| Pull_10_Changes/ (allocs/op) | 1,556 allocs | 1,555 allocs | 🟢 -0.06% |
| Pull_100_Changes/ (ns/op) | 3.47 ms | 3.48 ms | 🔴 +0.26% |
| Pull_100_Changes/ (B/op) | 699.63 KB | 699.46 KB | 🟢 -0.02% |
| Pull_100_Changes/ (allocs/op) | 6,111 allocs | 6,109 allocs | 🟢 -0.03% |
| Pull_1000_Changes/ (ns/op) | 10.21 ms | 10.61 ms | 🔴 +3.98% |
| Pull_1000_Changes/ (B/op) | 6.14 MB | 6.14 MB | ⚪ 0% |
| Pull_1000_Changes/ (allocs/op) | 52,967 allocs | 52,962 allocs | ⚪ 0% |
BenchmarkSnapshot
| Benchmark suite | Previous | Current | Change |
|---|---|---|---|
| Push_3KB_snapshot/ (ns/op) | 18.34 ms | 18.25 ms | 🟢 -0.49% |
| Push_3KB_snapshot/ (B/op) | 1.52 MB | 1.52 MB | 🟢 -0.16% |
| Push_3KB_snapshot/ (allocs/op) | 13,161 allocs | 13,153 allocs | 🟢 -0.06% |
| Push_30KB_snapshot/ (ns/op) | 155.26 ms | 158.14 ms | 🔴 +1.86% |
| Push_30KB_snapshot/ (B/op) | 18.80 MB | 18.90 MB | 🔴 +0.52% |
| Push_30KB_snapshot/ (allocs/op) | 185,759 allocs | 187,587 allocs | 🔴 +0.98% |
| Pull_3KB_snapshot/ (ns/op) | 4.02 ms | 4.31 ms | 🔴 +7.28% |
| Pull_3KB_snapshot/ (B/op) | 1.16 MB | 1.16 MB | ⚪ 0% |
| Pull_3KB_snapshot/ (allocs/op) | 15,263 allocs | 15,261 allocs | 🟢 -0.01% |
| Pull_30KB_snapshot/ (ns/op) | 14.97 ms | 16.95 ms | 🔴 +13.22% |
| Pull_30KB_snapshot/ (B/op) | 10.78 MB | 10.79 MB | 🔴 +0.05% |
| Pull_30KB_snapshot/ (allocs/op) | 142,457 allocs | 142,518 allocs | 🔴 +0.04% |
BenchmarkSplayTree
| Benchmark suite | Previous | Current | Change |
|---|---|---|---|
| stress_test_100000/ (ns/op) | 0.18 ns | 0.18 ns | 🔴 +3.80% |
| stress_test_100000/ (B/op) | 0.00 B | 0.00 B | ⚪ 0% |
| stress_test_100000/ (allocs/op) | 0 allocs | 0 allocs | ⚪ 0% |
| stress_test_200000/ (ns/op) | 0.36 ns | 0.37 ns | 🔴 +3.56% |
| stress_test_200000/ (B/op) | 0.00 B | 0.00 B | ⚪ 0% |
| stress_test_200000/ (allocs/op) | 0 allocs | 0 allocs | ⚪ 0% |
| stress_test_300000/ (ns/op) | 0.53 ns | 0.55 ns | 🔴 +3.29% |
| stress_test_300000/ (B/op) | 0.00 B | 0.00 B | ⚪ 0% |
| stress_test_300000/ (allocs/op) | 0 allocs | 0 allocs | ⚪ 0% |
| random_access_100000/ (ns/op) | 0.01 ns | 0.01 ns | 🔴 +1.21% |
| random_access_100000/ (B/op) | 0.00 B | 0.00 B | ⚪ 0% |
| random_access_100000/ (allocs/op) | 0 allocs | 0 allocs | ⚪ 0% |
| random_access_200000/ (ns/op) | 0.03 ns | 0.03 ns | 🟢 -3.42% |
| random_access_200000/ (B/op) | 0.00 B | 0.00 B | ⚪ 0% |
| random_access_200000/ (allocs/op) | 0 allocs | 0 allocs | ⚪ 0% |
| random_access_300000/ (ns/op) | 0.05 ns | 0.05 ns | 🟢 -0.30% |
| random_access_300000/ (B/op) | 0.00 B | 0.00 B | ⚪ 0% |
| random_access_300000/ (allocs/op) | 0 allocs | 0 allocs | ⚪ 0% |
| editing_trace_bench/ (ns/op) | 0.00 ns | 0.00 ns | 🔴 +19.14% |
| editing_trace_bench/ (B/op) | 0.00 B | 0.00 B | ⚪ 0% |
| editing_trace_bench/ (allocs/op) | 0 allocs | 0 allocs | ⚪ 0% |
BenchmarkSync
| Benchmark suite | Previous | Current | Change |
|---|---|---|---|
| memory_sync_10_test/ (ns/op) | 6680.00 ns | 7264.00 ns | 🔴 +8.74% |
| memory_sync_10_test/ (B/op) | 1.34 KB | 1.34 KB | ⚪ 0% |
| memory_sync_10_test/ (allocs/op) | 35 allocs | 35 allocs | ⚪ 0% |
| memory_sync_100_test/ (ns/op) | 54443.00 ns | 56024.00 ns | 🔴 +2.90% |
| memory_sync_100_test/ (B/op) | 9.54 KB | 9.51 KB | 🟢 -0.28% |
| memory_sync_100_test/ (allocs/op) | 269 allocs | 268 allocs | 🟢 -0.37% |
| memory_sync_1000_test/ (ns/op) | 604523.00 ns | 601818.00 ns | 🟢 -0.45% |
| memory_sync_1000_test/ (B/op) | 76.25 KB | 76.79 KB | 🔴 +0.70% |
| memory_sync_1000_test/ (allocs/op) | 2,121 allocs | 2,138 allocs | 🔴 +0.80% |
| memory_sync_10000_test/ (ns/op) | 8.13 ms | 7.89 ms | 🟢 -2.91% |
| memory_sync_10000_test/ (B/op) | 754.44 KB | 761.67 KB | 🔴 +0.96% |
| memory_sync_10000_test/ (allocs/op) | 20,386 allocs | 20,571 allocs | 🔴 +0.91% |
BenchmarkSyncConcurrency
| Benchmark suite | Previous | Current | Change |
|---|---|---|---|
| 1-100-10/ (ns/op) | 8.07 s | 8.04 s | 🟢 -0.46% |
| 1-100-10/ (B/op) | 4.33 GB | 4.32 GB | 🟢 -0.30% |
| 1-100-10/ (allocs/op) | 106,624,075 allocs | 106,529,729 allocs | 🟢 -0.09% |
| 100-100-10/ (ns/op) | 9.09 s | 9.27 s | 🔴 +1.92% |
| 100-100-10/ (B/op) | 4.43 GB | 4.48 GB | 🔴 +1.07% |
| 100-100-10/ (allocs/op) | 107,661,169 allocs | 108,019,859 allocs | 🔴 +0.33% |
| 300_100-10/ (ns/op) | 11.27 s | 11.36 s | 🔴 +0.73% |
| 300_100-10/ (B/op) | 4.67 GB | 4.67 GB | ⚪ 0% |
| 300_100-10/ (allocs/op) | 110,634,332 allocs | 110,618,614 allocs | 🟢 -0.01% |
BenchmarkTextEditing
| Benchmark suite | Previous | Current | Change |
|---|---|---|---|
| (ns/op) | 5.29 s | 5.29 s | ⚪ 0% |
| (B/op) | 3.88 GB | 3.88 GB | ⚪ 0% |
| (allocs/op) | 20,100,336 allocs | 20,100,247 allocs | ⚪ 0% |
BenchmarkTree
| Benchmark suite | Previous | Current | Change |
|---|---|---|---|
| 10000_vertices_to_protobuf/ (ns/op) | 4.86 ms | 4.85 ms | 🟢 -0.24% |
| 10000_vertices_to_protobuf/ (B/op) | 6.68 MB | 6.68 MB | ⚪ 0% |
| 10000_vertices_to_protobuf/ (allocs/op) | 90,026 allocs | 90,026 allocs | ⚪ 0% |
| 10000_vertices_from_protobuf/ (ns/op) | 234.70 ms | 230.04 ms | 🟢 -1.99% |
| 10000_vertices_from_protobuf/ (B/op) | 442.15 MB | 442.15 MB | ⚪ 0% |
| 10000_vertices_from_protobuf/ (allocs/op) | 280,046 allocs | 280,047 allocs | ⚪ 0% |
| 20000_vertices_to_protobuf/ (ns/op) | 10.68 ms | 10.40 ms | 🟢 -2.65% |
| 20000_vertices_to_protobuf/ (B/op) | 13.53 MB | 13.53 MB | ⚪ 0% |
| 20000_vertices_to_protobuf/ (allocs/op) | 180,029 allocs | 180,029 allocs | ⚪ 0% |
| 20000_vertices_from_protobuf/ (ns/op) | 899.53 ms | 891.44 ms | 🟢 -0.90% |
| 20000_vertices_from_protobuf/ (B/op) | 1.70 GB | 1.70 GB | ⚪ 0% |
| 20000_vertices_from_protobuf/ (allocs/op) | 560,078 allocs | 560,078 allocs | ⚪ 0% |
| 30000_vertices_to_protobuf/ (ns/op) | 16.29 ms | 15.98 ms | 🟢 -1.90% |
| 30000_vertices_to_protobuf/ (B/op) | 19.94 MB | 19.94 MB | ⚪ 0% |
| 30000_vertices_to_protobuf/ (allocs/op) | 270,030 allocs | 270,030 allocs | ⚪ 0% |
| 30000_vertices_from_protobuf/ (ns/op) | 2.07 s | 2.05 s | 🟢 -0.71% |
| 30000_vertices_from_protobuf/ (B/op) | 3.75 GB | 3.75 GB | ⚪ 0% |
| 30000_vertices_from_protobuf/ (allocs/op) | 840,281 allocs | 840,129 allocs | 🟢 -0.02% |
BenchmarkVersionVector
| Benchmark suite | Previous | Current | Change |
|---|---|---|---|
| clients_10/ (ns/op) | 125.60 ms | 125.67 ms | 🔴 +0.06% |
| clients_10/ (1_changepack(bytes)) | 185.00 B | 185.00 B | ⚪ 0% |
| clients_10/ (2_snapshot(bytes)) | 399.00 B | 399.00 B | ⚪ 0% |
| clients_10/ (3_pushpull(ms)) | 4.00 ms | 4.00 ms | ⚪ 0% |
| clients_10/ (4_attach(ms)) | 5.00 ms | 5.00 ms | ⚪ 0% |
| clients_10/ (5_changepack_after_detach(bytes)) | 230.00 B | 230.00 B | ⚪ 0% |
| clients_10/ (6_snapshot_after_detach(bytes)) | 156.00 B | 156.00 B | ⚪ 0% |
| clients_10/ (7_pushpull_after_detach(ms)) | 4.00 ms | 4.00 ms | ⚪ 0% |
| clients_10/ (B/op) | 8.25 MB | 8.66 MB | 🔴 +5.04% |
| clients_10/ (allocs/op) | 65,738 allocs | 65,776 allocs | 🔴 +0.06% |
| clients_100/ (ns/op) | 1.03 s | 1.04 s | 🔴 +0.59% |
| clients_100/ (1_changepack(bytes)) | 185.00 B | 185.00 B | ⚪ 0% |
| clients_100/ (2_snapshot(bytes)) | 3.10 KB | 3.10 KB | ⚪ 0% |
| clients_100/ (3_pushpull(ms)) | 4.00 ms | 4.00 ms | ⚪ 0% |
| clients_100/ (4_attach(ms)) | 6.00 ms | 6.00 ms | ⚪ 0% |
| clients_100/ (5_changepack_after_detach(bytes)) | 231.00 B | 231.00 B | ⚪ 0% |
| clients_100/ (6_snapshot_after_detach(bytes)) | 156.00 B | 156.00 B | ⚪ 0% |
| clients_100/ (7_pushpull_after_detach(ms)) | 5.00 ms | 5.00 ms | ⚪ 0% |
| clients_100/ (B/op) | 77.15 MB | 78.02 MB | 🔴 +1.13% |
| clients_100/ (allocs/op) | 648,302 allocs | 648,354 allocs | ⚪ 0% |
| clients_1000/ (ns/op) | 11.98 s | 11.92 s | 🟢 -0.44% |
| clients_1000/ (1_changepack(bytes)) | 186.00 B | 186.00 B | ⚪ 0% |
| clients_1000/ (2_snapshot(bytes)) | 30.10 KB | 30.10 KB | ⚪ 0% |
| clients_1000/ (3_pushpull(ms)) | 8.00 ms | 5.00 ms | 🟢 -37.50% |
| clients_1000/ (4_attach(ms)) | 19.00 ms | 19.00 ms | ⚪ 0% |
| clients_1000/ (5_changepack_after_detach(bytes)) | 231.00 B | 231.00 B | ⚪ 0% |
| clients_1000/ (6_snapshot_after_detach(bytes)) | 156.00 B | 156.00 B | ⚪ 0% |
| clients_1000/ (7_pushpull_after_detach(ms)) | 5.00 ms | 5.00 ms | ⚪ 0% |
| clients_1000/ (B/op) | 1.50 GB | 1.50 GB | 🔴 +0.03% |
| clients_1000/ (allocs/op) | 18,345,242 allocs | 18,347,284 allocs | 🔴 +0.01% |
BenchmarkWebhook
| Benchmark suite | Previous | Current | Change |
|---|---|---|---|
| Send_10_Webhooks_to_10_Endpoints/ (ns/op) | 12.37 ms | 12.38 ms | 🔴 +0.06% |
| Send_10_Webhooks_to_10_Endpoints/ (B/op) | 808.52 KB | 808.34 KB | 🟢 -0.02% |
| Send_10_Webhooks_to_10_Endpoints/ (allocs/op) | 10,116 allocs | 10,116 allocs | ⚪ 0% |
| Send_100_Webhooks_to_10_Endpoints/ (ns/op) | 123.41 ms | 122.72 ms | 🟢 -0.56% |
| Send_100_Webhooks_to_10_Endpoints/ (B/op) | 8.08 MB | 8.08 MB | 🟢 -0.02% |
| Send_100_Webhooks_to_10_Endpoints/ (allocs/op) | 101,160 allocs | 101,150 allocs | ⚪ 0% |
| Send_10_Webhooks_to_100_Endpoints/ (ns/op) | 125.17 ms | 125.46 ms | 🔴 +0.24% |
| Send_10_Webhooks_to_100_Endpoints/ (B/op) | 8.28 MB | 8.28 MB | ⚪ 0% |
| Send_10_Webhooks_to_100_Endpoints/ (allocs/op) | 102,543 allocs | 102,537 allocs | ⚪ 0% |
| Send_100_Webhooks_to_100_Endpoints/ (ns/op) | 1.26 s | 1.25 s | 🟢 -1.27% |
| Send_100_Webhooks_to_100_Endpoints/ (B/op) | 83.38 MB | 82.34 MB | 🟢 -1.25% |
| Send_100_Webhooks_to_100_Endpoints/ (allocs/op) | 1,022,759 allocs | 1,022,305 allocs | 🟢 -0.04% |
| Send_10_Webhooks_to_1000_Endpoints/ (ns/op) | 2.77 s | 2.76 s | 🟢 -0.11% |
| Send_10_Webhooks_to_1000_Endpoints/ (B/op) | 209.76 MB | 209.63 MB | 🟢 -0.06% |
| Send_10_Webhooks_to_1000_Endpoints/ (allocs/op) | 1,716,884 allocs | 1,716,718 allocs | ⚪ 0% |
BenchmarkWebhookWithLimit
| Benchmark suite | Previous | Current | Change |
|---|---|---|---|
| Send_10_Webhooks_to_10_Endpoints_with_limit/ (ns/op) | 3.79 ms | 3.76 ms | 🟢 -0.64% |
| Send_10_Webhooks_to_10_Endpoints_with_limit/ (B/op) | 306.38 KB | 306.47 KB | 🔴 +0.03% |
| Send_10_Webhooks_to_10_Endpoints_with_limit/ (allocs/op) | 2,913 allocs | 2,913 allocs | ⚪ 0% |
| Send_100_Webhooks_to_10_Endpoints_with_limit/ (ns/op) | 4.04 ms | 4.04 ms | 🔴 +0.04% |
| Send_100_Webhooks_to_10_Endpoints_with_limit/ (B/op) | 428.87 KB | 428.81 KB | 🟢 -0.01% |
| Send_100_Webhooks_to_10_Endpoints_with_limit/ (allocs/op) | 4,713 allocs | 4,713 allocs | ⚪ 0% |
| Send_10_Webhooks_to_100_Endpoints_with_limit/ (ns/op) | 37.41 ms | 37.89 ms | 🔴 +1.29% |
| Send_10_Webhooks_to_100_Endpoints_with_limit/ (B/op) | 3.08 MB | 3.08 MB | ⚪ 0% |
| Send_10_Webhooks_to_100_Endpoints_with_limit/ (allocs/op) | 29,169 allocs | 29,172 allocs | 🔴 +0.01% |
| Send_100_Webhooks_to_100_Endpoints_with_limit/ (ns/op) | 40.94 ms | 40.48 ms | 🟢 -1.14% |
| Send_100_Webhooks_to_100_Endpoints_with_limit/ (B/op) | 4.37 MB | 4.37 MB | 🟢 -0.02% |
| Send_100_Webhooks_to_100_Endpoints_with_limit/ (allocs/op) | 47,199 allocs | 47,196 allocs | ⚪ 0% |
| Send_10_Webhooks_to_1000_Endpoints_with_limit/ (ns/op) | 365.99 ms | 367.65 ms | 🔴 +0.45% |
| Send_10_Webhooks_to_1000_Endpoints_with_limit/ (B/op) | 44.52 MB | 44.52 MB | 🟢 -0.01% |
| Send_10_Webhooks_to_1000_Endpoints_with_limit/ (allocs/op) | 380,176 allocs | 380,143 allocs | ⚪ 0% |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
pkg/document/crdt/tree.go (1)
602-608: Update method comment to distinguish from Tree.Marshal.The implementation is correct, but the comment is identical to
Tree.Marshalwhich could cause confusion.-// Marshal returns the JSON encoding of this Tree. +// Marshal returns the JSON encoding of this TreeNode with creation time prefix.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
pkg/document/crdt/tree.go(7 hunks)pkg/index/tree.go(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- pkg/index/tree.go
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: bench
- GitHub Check: build
- GitHub Check: load-test
- GitHub Check: complex-test
🔇 Additional comments (9)
pkg/document/crdt/tree.go (9)
154-161: LGTM - Well-designed optional parameter handling.The variadic parameter approach maintains backward compatibility while enabling the new functionality to include removed nodes. The implementation correctly delegates to the underlying index layer.
242-245: LGTM - Clean delegation pattern.The method provides a clear interface for clearing children and properly propagates errors from the underlying index operation.
867-899: LGTM - Excellent refactoring for modularity.The replacement of
collectBetweenwithcollectDeleteAndMergeand the delegation to specializedRemoveNodeandMergeNodemethods significantly improves code organization and maintainability. The error handling and GC pair management are properly preserved.
948-985: LGTM - Sophisticated node classification logic.The stack-based approach effectively distinguishes between nodes that should be deleted (both start and end tokens deleted) versus merged (only end token deleted). The implementation properly handles text nodes as always deletable and uses appropriate error handling.
The reversal of the merges slice suggests correct processing order for tree operations.
987-1006: LGTM - Proper CRDT conflict resolution.The method correctly implements actor-based conflict resolution using version vectors and Lamport timestamps. The dual handling of empty vs. non-empty version vectors appropriately covers both local and remote editing scenarios.
1008-1050: LGTM - Robust merge operation implementation.The method properly handles the complex merge scenario by checking type compatibility, moving children between nodes, and maintaining proper CRDT semantics with version vector conflict resolution. The use of
NextSiblingExtendedand proper child manipulation ensures tree integrity.
1102-1123: LGTM - Well-designed traversal function.The iterative callback-based traversal correctly implements position-to-position iteration with proper termination conditions. The safety comment about not modifying the tree during traversal is valuable for preventing corruption.
1125-1159: LGTM - Comprehensive tree position advancement.The function correctly handles all traversal cases (parent/child relationships, text vs. element nodes, sibling navigation) with proper boundary checks and parent existence validation. The logic appropriately distinguishes between different node types and maintains tree structure integrity.
689-689: Minor formatting change noted.This appears to be a formatting adjustment in the JSON serialization logic. The change doesn't affect functionality.
There was a problem hiding this comment.
k6 Load Test 📊
This is a comparison result between the previous(851db0e) and the current commit(4728958).
Change
| Metric | Previous | Current | Change |
|---|---|---|---|
| transaction_time.avg | 6808.41 | 6804.12 | 🟢 -0.06% |
| http_req_duration.avg | 82.85 | 83.33 | 🔴 +0.58% |
| http_req_duration.p(95) | 323.05 | 333.46 | 🔴 +3.22% |
| iteration_duration.avg | 6808.46 | 6804.17 | 🟢 -0.06% |
| data_received.count | 1,094,126,058 | 1,087,568,743 | 🟢 -0.60% |
| data_sent.count | 59,852,313 | 59,806,956 | 🟢 -0.08% |
Result
script: test/k6/presence.ts
scenarios: (100.00%) 1 scenario, 500 max VUs, 3m0s max duration (incl. graceful stop):
* default: Up to 500 looping VUs for 2m30s over 3 stages (gracefulRampDown: 30s, gracefulStop: 30s)
█ THRESHOLDS
active_clients_time
✓ 'p(95)<500' p(95)=88.75ms
attach_documents_time
✓ 'p(95)<10000' p(95)=500.75ms
deactivate_clients_time
✓ 'p(95)<10000' p(95)=569.75ms
http_req_duration
✓ 'p(95)<10000' p(95)=333.45ms
pushpulls_time
✓ 'p(95)<1000' p(95)=272.54ms
transaction_success_rate
✓ 'rate>0.99' rate=100.00%
transaction_time
✓ 'p(95)<30000' p(95)=7.74s
█ TOTAL RESULTS
checks_total.......................: 127136 823.807862/s
checks_succeeded...................: 100.00% 127136 out of 127136
checks_failed......................: 0.00% 0 out of 127136
✓ status is 200
✓ response has valid body
CUSTOM
active_clients..........................................................: 7946 51.487991/s
active_clients_success_rate.............................................: 100.00% 7946 out of 7946
active_clients_time.....................................................: avg=18.79ms min=1ms med=5ms max=540ms p(90)=55ms p(95)=88.75ms
attach_documents........................................................: 7946 51.487991/s
attach_documents_success_rate...........................................: 100.00% 7946 out of 7946
attach_documents_time...................................................: avg=156.4ms min=3ms med=116ms max=863ms p(90)=386.5ms p(95)=500.75ms
deactivate_clients......................................................: 7946 51.487991/s
deactivate_clients_success_rate.........................................: 100.00% 7946 out of 7946
deactivate_clients_time.................................................: avg=182.87ms min=4ms med=133ms max=968ms p(90)=458ms p(95)=569.75ms
pushpulls...............................................................: 39730 257.439957/s
pushpulls_success_rate..................................................: 100.00% 39730 out of 39730
pushpulls_time..........................................................: avg=74.54ms min=1ms med=33ms max=1.01s p(90)=203ms p(95)=272.54ms
transaction_success_rate................................................: 100.00% 7946 out of 7946
transaction_time........................................................: avg=6.8s min=6.02s med=6.87s max=9.12s p(90)=7.52s p(95)=7.74s
HTTP
http_req_duration.......................................................: avg=83.33ms min=930.89µs med=26.83ms max=967.49ms p(90)=237.97ms p(95)=333.45ms
{ expected_response:true }............................................: avg=83.33ms min=930.89µs med=26.83ms max=967.49ms p(90)=237.97ms p(95)=333.45ms
http_req_failed.........................................................: 0.00% 0 out of 63568
http_reqs...............................................................: 63568 411.903931/s
EXECUTION
iteration_duration......................................................: avg=6.8s min=6.02s med=6.87s max=9.12s p(90)=7.52s p(95)=7.74s
iterations..............................................................: 7946 51.487991/s
vus.....................................................................: 11 min=8 max=500
vus_max.................................................................: 500 min=500 max=500
NETWORK
data_received...........................................................: 1.1 GB 7.0 MB/s
data_sent...............................................................: 60 MB 388 kB/s
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
pkg/document/crdt/tree.go (3)
602-608: Fix the method comment.The comment incorrectly states "Marshal returns the JSON encoding of this Tree" when it should refer to the TreeNode.
-// Marshal returns the JSON encoding of this Tree. +// Marshal returns the JSON encoding of this TreeNode.
948-985: Complex but well-implemented node classification logic.The stack-based algorithm correctly identifies deletable vs mergeable nodes based on whether both start/end tokens or only the end token fall within the range. However, the intricate logic would benefit from more detailed comments explaining the classification criteria.
Consider adding detailed comments to explain the stack-based algorithm:
+ // Stack-based algorithm to classify nodes: + // - Text nodes: always deletable + // - Element nodes: deletable if both start and end tokens are in range, + // mergeable if only the end token is in range err := traverseInTreePos(fromParent, fromLeft, toParent, toLeft, func(parent, left *TreeNode) error {
1008-1050: LGTM! Comprehensive node merging implementation.The method correctly handles type validation, child migration, and cleanup. The logic for finding mergeable siblings and moving their children is sound. However, consider the performance implications of the potentially long sibling traversal chain.
For performance optimization, consider adding an early termination condition if the sibling traversal chain becomes too long, to prevent potential performance issues in deeply nested or wide trees.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
pkg/document/crdt/tree.go(7 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: load-test
- GitHub Check: bench
- GitHub Check: complex-test
- GitHub Check: build
🔇 Additional comments (5)
pkg/document/crdt/tree.go (5)
154-161: LGTM! Well-designed backward-compatible API enhancement.The variadic parameter approach maintains backward compatibility while adding the ability to include removed nodes when needed. The implementation correctly handles the optional parameter.
242-245: LGTM! Clean utility method for clearing children.The method provides a clear and intuitive API for removing all children from a node, properly delegating to the underlying index implementation.
867-899: LGTM! Excellent refactoring that improves modularity.The separation of concerns between node collection (
collectDeleteAndMerge) and node operations (RemoveNode/MergeNode) makes the code more maintainable and easier to understand. The error handling is consistent and appropriate.
987-1006: LGTM! Well-encapsulated node removal logic.The method properly handles version vector validation and provides clear semantics for when a node can be removed. The separation of concerns improves code organization.
689-689: LGTM! Correct JSON formatting fix.The added comma ensures proper JSON structure when attributes are present.
| // callback must not modify the tree by `pkg/index/tree.go L256` | ||
| func traverseInTreePos(fromParent, fromLeft, toParent, toLeft *TreeNode, | ||
| callback func(parent *TreeNode, left *TreeNode) error, | ||
| ) error { | ||
| isEnd := func(p1, l1, p2, l2 *TreeNode) bool { | ||
| return p1.ID().Equals(p2.ID()) && l1.ID().Equals(l2.ID()) | ||
| } | ||
|
|
||
| var ok bool | ||
| stk := []*index.Node[*TreeNode]{} | ||
| for { | ||
| if isEnd(fromParent, fromLeft, toParent, toLeft) { | ||
| break | ||
| } | ||
| if fromParent, fromLeft, ok = nextTreePos(fromParent, fromLeft, &stk); !ok { | ||
| break | ||
| } | ||
| if err := callback(fromParent, fromLeft); err != nil { | ||
| break | ||
| } | ||
| } | ||
| return nil | ||
| } | ||
|
|
||
| func nextTreePos(parent, left *TreeNode, stk *[]*index.Node[*TreeNode]) (*TreeNode, *TreeNode, bool) { | ||
| var nextNode *TreeNode = nil | ||
|
|
||
| if parent == left { | ||
| children := slices.Clone(parent.Index.GetChildren()) | ||
| *stk = append(children, (*stk)...) | ||
| if len(*stk) != 0 { | ||
| nextNode = (*stk)[0].Value | ||
| } | ||
|
|
||
| if nextNode != nil && nextNode.Index.Parent == parent.Index { | ||
| *stk = (*stk)[1:] | ||
| if nextNode.IsText() { | ||
| return parent, nextNode, true | ||
| } else { | ||
| return nextNode, nextNode, true | ||
| } | ||
| } | ||
| if parent.Index.Parent == nil { | ||
| return nil, nil, false | ||
| } | ||
| return parent.Index.Parent.Value, parent, true | ||
| } | ||
|
|
||
| if len(*stk) != 0 { | ||
| nextNode = (*stk)[0].Value | ||
| } else { | ||
| offset := parent.Index.OffsetOfChild(left.Index) | ||
| children := slices.Clone(parent.Index.GetChildren()[offset+1:]) | ||
| *stk = append(children, (*stk)...) | ||
| if len(*stk) != 0 { | ||
| nextNode = (*stk)[0].Value | ||
| } | ||
| } | ||
|
|
||
| if nextNode != nil && nextNode.Index.Parent == parent.Index { | ||
| *stk = (*stk)[1:] | ||
| if nextNode.IsText() { | ||
| return parent, nextNode, true | ||
| } else { | ||
| return nextNode, nextNode, true | ||
| } | ||
| } | ||
| if parent.Index.Parent == nil { | ||
| return nil, nil, false | ||
| } | ||
| return parent.Index.Parent.Value, parent, true | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Complex but necessary traversal implementation.
The traversal logic correctly handles the nuanced differences between TreePos positions and properly advances through nested tree structures. However, this complex algorithm critically needs comprehensive documentation.
This intricate traversal logic requires detailed documentation to explain:
- The difference between parent==left vs parent!=left positions
- How the stack manages nested element traversal
- The termination conditions and edge cases
+// traverseInTreePos traverses nodes between two TreePos positions.
+// The algorithm handles two types of positions:
+// - parent==left: represents the start of an element node
+// - parent!=left: represents a position after a specific child node
+// Uses a stack to manage traversal through nested structures.
func traverseInTreePos(fromParent, fromLeft, toParent, toLeft *TreeNode,📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // callback must not modify the tree by `pkg/index/tree.go L256` | |
| func traverseInTreePos(fromParent, fromLeft, toParent, toLeft *TreeNode, | |
| callback func(parent *TreeNode, left *TreeNode) error, | |
| ) error { | |
| isEnd := func(p1, l1, p2, l2 *TreeNode) bool { | |
| return p1.ID().Equals(p2.ID()) && l1.ID().Equals(l2.ID()) | |
| } | |
| var ok bool | |
| stk := []*index.Node[*TreeNode]{} | |
| for { | |
| if isEnd(fromParent, fromLeft, toParent, toLeft) { | |
| break | |
| } | |
| if fromParent, fromLeft, ok = nextTreePos(fromParent, fromLeft, &stk); !ok { | |
| break | |
| } | |
| if err := callback(fromParent, fromLeft); err != nil { | |
| break | |
| } | |
| } | |
| return nil | |
| } | |
| func nextTreePos(parent, left *TreeNode, stk *[]*index.Node[*TreeNode]) (*TreeNode, *TreeNode, bool) { | |
| var nextNode *TreeNode = nil | |
| if parent == left { | |
| children := slices.Clone(parent.Index.GetChildren()) | |
| *stk = append(children, (*stk)...) | |
| if len(*stk) != 0 { | |
| nextNode = (*stk)[0].Value | |
| } | |
| if nextNode != nil && nextNode.Index.Parent == parent.Index { | |
| *stk = (*stk)[1:] | |
| if nextNode.IsText() { | |
| return parent, nextNode, true | |
| } else { | |
| return nextNode, nextNode, true | |
| } | |
| } | |
| if parent.Index.Parent == nil { | |
| return nil, nil, false | |
| } | |
| return parent.Index.Parent.Value, parent, true | |
| } | |
| if len(*stk) != 0 { | |
| nextNode = (*stk)[0].Value | |
| } else { | |
| offset := parent.Index.OffsetOfChild(left.Index) | |
| children := slices.Clone(parent.Index.GetChildren()[offset+1:]) | |
| *stk = append(children, (*stk)...) | |
| if len(*stk) != 0 { | |
| nextNode = (*stk)[0].Value | |
| } | |
| } | |
| if nextNode != nil && nextNode.Index.Parent == parent.Index { | |
| *stk = (*stk)[1:] | |
| if nextNode.IsText() { | |
| return parent, nextNode, true | |
| } else { | |
| return nextNode, nextNode, true | |
| } | |
| } | |
| if parent.Index.Parent == nil { | |
| return nil, nil, false | |
| } | |
| return parent.Index.Parent.Value, parent, true | |
| } | |
| // traverseInTreePos traverses nodes between two TreePos positions. | |
| // The algorithm handles two types of positions: | |
| // - parent==left: represents the start of an element node | |
| // - parent!=left: represents a position after a specific child node | |
| // Uses a stack to manage traversal through nested structures. | |
| // callback must not modify the tree by `pkg/index/tree.go L256` | |
| func traverseInTreePos(fromParent, fromLeft, toParent, toLeft *TreeNode, | |
| callback func(parent *TreeNode, left *TreeNode) error, | |
| ) error { | |
| isEnd := func(p1, l1, p2, l2 *TreeNode) bool { | |
| return p1.ID().Equals(p2.ID()) && l1.ID().Equals(l2.ID()) | |
| } | |
| var ok bool | |
| stk := []*index.Node[*TreeNode]{} | |
| for { | |
| if isEnd(fromParent, fromLeft, toParent, toLeft) { | |
| break | |
| } | |
| if fromParent, fromLeft, ok = nextTreePos(fromParent, fromLeft, &stk); !ok { | |
| break | |
| } | |
| if err := callback(fromParent, fromLeft); err != nil { | |
| break | |
| } | |
| } | |
| return nil | |
| } |
🤖 Prompt for AI Agents
In pkg/document/crdt/tree.go around lines 1102 to 1173, the traverseInTreePos
and nextTreePos functions implement a complex tree traversal that distinguishes
between cases where parent equals left and where they differ, uses a stack to
manage nested traversal, and has specific termination conditions. Add detailed
comments explaining the significance of the parent==left versus parent!=left
cases, how the stack is used to track and traverse nested children nodes, and
the conditions under which the traversal ends or backtracks. This documentation
should clarify the traversal flow and edge cases to aid future maintainers.
hackerwins
force-pushed
the
main
branch
3 times, most recently
from
e40d5e0 to
b4a654a
Compare
hackerwins
force-pushed
the
main
branch
2 times, most recently
from
6f9ef37 to
df0b450
Compare
What this PR does / why we need it:
This PR reimplements the logic of Tree.Edit—specifically its deletion, removal, and traversal components—to improve clarity and future extensibility.
The updated implementation passes all test cases currently covered by the previous version, ensuring full backward compatibility and convergence correctness.
Although this PR additionally passes several new test cases, it does not yet address most of the known convergence issues that remain unsolved in the current implementation.
Refined
traverseInTreePosLogicFor example, after removing
"a"from<p>"12""a""34"</p>,the new traversal yields
"12","a","34"instead of just"12","34".Reworked Node Removal Logic
traverseInPosRangelogic to simplify control flow.canDeleteandtoBeRemovedshandling is decoupled:toBeRemoveds.canDeletebefore actually removing nodes.Before:
After:
Motivation
The current logic for
Tree.Edithas several limitations in handling merged nodes, and overlapping edits. These changes simplify reasoning and improve consistency while enabling powerful future capabilities like:$
Which issue(s) this PR fixes:
Fixes #
Special notes for your reviewer:
Does this PR introduce a user-facing change?:
Additional documentation:
Checklist:
Summary by CodeRabbit
New Features
Bug Fixes
Refactor