Remove deprecated MinSyncedTicket and MaxCreatedAtMapByActor#1208
Conversation
WalkthroughThis set of changes removes legacy synchronization and garbage collection fields and logic, specifically the deprecated Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Server
participant CRDT
Client->>Server: PushPull (ChangePack with VersionVector)
Server->>CRDT: Apply changes (no MinSyncedTicket/maxCreatedAtMapByActor)
CRDT-->>Server: Updated document state
Server-->>Client: Response (ChangePack, VersionVector)
Possibly related PRs
Suggested reviewers
Tip ⚡️ Faster reviews with caching
Enjoy the performance boost—your workflow just got faster. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (4)
💤 Files with no reviewable changes (3)
✅ Files skipped from review due to trivial changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (3)
✨ Finishing Touches
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. 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 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: 0
🧹 Nitpick comments (1)
api/yorkie/v1/resources.proto (1)
47-47: Deprecated Field Annotation formin_synced_ticket:
The deprecation comment on themin_synced_ticketfield clearly signals that this legacy field should no longer be used. This aligns with the migration away from Lamport timestamps. Consider, if supported by your proto version and team standards, using a formal deprecation option (for example, a field option like(deprecated) = true) to enhance tooling support.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (24)
api/converter/converter_test.go(0 hunks)api/converter/from_bytes.go(1 hunks)api/converter/from_pb.go(1 hunks)api/converter/to_pb.go(3 hunks)api/yorkie/v1/resources.proto(5 hunks)pkg/document/change/pack.go(0 hunks)pkg/document/crdt/rga_tree_split.go(8 hunks)pkg/document/crdt/root_test.go(4 hunks)pkg/document/crdt/text.go(4 hunks)pkg/document/crdt/text_test.go(2 hunks)pkg/document/crdt/tree.go(16 hunks)pkg/document/crdt/tree_test.go(1 hunks)pkg/document/document_test.go(0 hunks)pkg/document/json/text.go(2 hunks)pkg/document/json/tree.go(3 hunks)pkg/document/operations/edit.go(2 hunks)pkg/document/operations/style.go(2 hunks)pkg/document/operations/tree_edit.go(2 hunks)pkg/document/operations/tree_style.go(3 hunks)server/backend/database/database.go(0 hunks)server/backend/database/memory/database.go(0 hunks)server/backend/database/mongo/client.go(0 hunks)server/packs/packs.go(0 hunks)server/packs/serverpacks.go(1 hunks)
💤 Files with no reviewable changes (7)
- server/packs/packs.go
- pkg/document/document_test.go
- pkg/document/change/pack.go
- server/backend/database/memory/database.go
- server/backend/database/mongo/client.go
- api/converter/converter_test.go
- server/backend/database/database.go
🧰 Additional context used
🧬 Code Definitions (6)
api/converter/to_pb.go (3)
api/yorkie/v1/resources.pb.go (6)
Operation_Style(2890-2901)Operation_Style(2916-2916)Operation_Style(2931-2933)Operation_TreeStyle(3135-3147)Operation_TreeStyle(3162-3162)Operation_TreeStyle(3177-3179)pkg/document/operations/style.go (1)
Style(25-40)pkg/document/operations/tree_style.go (1)
TreeStyle(25-43)
pkg/document/operations/tree_edit.go (1)
pkg/document/operations/edit.go (1)
Edit(26-45)
pkg/document/crdt/tree_test.go (3)
test/helper/helper.go (1)
TimeT(150-152)pkg/document/crdt/tree.go (1)
ToXML(1271-1273)pkg/index/tree.go (1)
ToXML(221-234)
pkg/document/crdt/rga_tree_split.go (3)
pkg/document/operations/remove.go (1)
Remove(25-35)pkg/document/time/ticket.go (1)
Ticket(54-61)pkg/document/crdt/gc.go (1)
GCPair(23-26)
pkg/document/operations/tree_style.go (1)
pkg/document/operations/style.go (1)
Style(25-40)
pkg/document/crdt/tree.go (6)
pkg/document/json/tree.go (2)
TreeNode(48-63)Tree(67-71)pkg/document/time/ticket.go (1)
Ticket(54-61)pkg/document/operations/edit.go (1)
Edit(26-45)pkg/document/crdt/gc.go (1)
GCPair(23-26)pkg/document/time/version_vector.go (1)
VersionVector(31-31)pkg/document/operations/style.go (1)
Style(25-40)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: complex-test
- GitHub Check: build
- GitHub Check: bench
🔇 Additional comments (88)
api/yorkie/v1/resources.proto (4)
99-99: Deprecated Annotation inEditMessage:
Thecreated_at_map_by_actorfield in theEditmessage is now marked as deprecated, which is consistent with cleaning up legacy functionality. Ensure that any consumers of this API update their code accordingly and that the deprecation is clearly documented in the changelogs.
120-120: Deprecated Annotation inStyleMessage:
The deprecation comment added to thecreated_at_map_by_actorfield within theStylemessage (line 120) conforms to the overall cleanup objectives. No further changes are needed here as long as the documentation reflects this deprecation.
131-131: Deprecated Annotation inTreeEditMessage:
Thecreated_at_map_by_actorfield in theTreeEditmessage is now annotated as deprecated (line 131). This change is consistent with the intention to remove support for legacy fields. Verify that any internal or external consumers are aware of this deprecation.
143-143: Deprecated Annotation inTreeStyleMessage:
The deprecation comment oncreated_at_map_by_actorin theTreeStylemessage (line 143) appropriately marks this field as legacy. This change supports the migration to Version Vectors while maintaining backward compatibility during the transition.server/packs/serverpacks.go (1)
121-127: Clean removal of MinSyncedTicket field from ChangePack constructionThe code properly removes the MinSyncedTicket field from the api.ChangePack object creation, which aligns with the PR objective to remove legacy fields after the Version Vector migration.
api/converter/to_pb.go (5)
163-170: Properly removed MinSyncedTicket field from ChangePack conversionThis change correctly removes the MinSyncedTicket field from the api.ChangePack construction in the ToChangePack method, which aligns with the PR's objective to clean up legacy fields.
364-370: Removed maxCreatedAtMapByActor field from Edit operation conversionThe Edit operation converter no longer includes the maxCreatedAtMapByActor field in the generated protobuf object, consistent with the removal of this legacy field across the codebase.
377-382: Removed maxCreatedAtMapByActor field from Style operation conversionThe Style operation converter no longer includes the maxCreatedAtMapByActor field in the generated protobuf object, consistent with the removal of this legacy field across the codebase.
404-410: Removed maxCreatedAtMapByActor field from TreeEdit operation conversionThe TreeEdit operation converter no longer includes the maxCreatedAtMapByActor field in the generated protobuf object, consistent with the removal of this legacy field across the codebase.
417-423: Removed maxCreatedAtMapByActor field from TreeStyle operation conversionThe TreeStyle operation converter no longer includes the maxCreatedAtMapByActor field in the generated protobuf object, consistent with the removal of this legacy field across the codebase.
pkg/document/operations/tree_edit.go (2)
55-62: Removed maxCreatedAtMapByActor field from TreeEdit struct constructorThe constructor properly handles the removal of the maxCreatedAtMapByActor field from the TreeEdit struct, which aligns with the PR's objective to clean up legacy fields after the Version Vector migration.
86-114: Removed maxCreatedAtMapByActor from TreeEdit.Execute methodThe Edit method now properly omits the maxCreatedAtMapByActor parameter when calling obj.Edit, which is consistent with the removal of this legacy field from the object's method signatures.
api/converter/from_bytes.go (1)
337-337: Removed maxCreatedAt parameter from textNode.Remove callThe Remove method call no longer includes the time.MaxTicket parameter, which aligns with the PR's objective to clean up legacy parameters related to maximum creation timestamps.
pkg/document/crdt/tree_test.go (5)
408-408: Simplified return value handling for StyleByIndexThe code now captures only the error return value from the
StyleByIndexmethod call, discarding the second return value (likely the maxCreatedAtMapByActor which has been removed as part of the version vector migration cleanup).
413-413: Return value simplification consistent with API changesThis change aligns with the removal of the maxCreatedAtMapByActor return value from StyleByIndex.
418-418: Simplified return values for StyleByIndex callThe change consistently removes the unnecessary return value capture across all method calls.
423-423: Consistent API call pattern for StyleByIndexContinues the pattern of simplifying the return value handling across the codebase.
429-429: Standardized StyleByIndex return handlingThis final change ensures all StyleByIndex calls in this test file have consistent return value handling, completing the cleanup.
pkg/document/json/text.go (2)
76-83: Simplified Edit method call with removal of maxCreatedAtMapByActorThe Edit method now correctly omits capturing the previously returned maxCreatedAtMapByActor value, aligning with the broader codebase cleanup after the Version Vector migration.
115-121: Streamlined Style method return value handlingThe Style method call follows the same pattern of removing maxCreatedAtMapByActor from the return values, maintaining consistency with the Edit method changes.
pkg/document/crdt/root_test.go (9)
68-69: Updated Edit method call following API simplificationThe call has been updated to match the new Edit method signature that no longer includes the maxCreatedAtMapByActor parameter, while maintaining the correct error checking.
75-77: Consistent Edit method call updateThis change follows the same pattern, removing the maxCreatedAtMapByActor parameter in the Edit method call.
82-84: API call alignment for content removal operationsThe Edit method call for removing content has been updated to match the new signature, maintaining consistency with other calls.
89-91: Standardized Edit method call for delete operationsEnsures consistent handling of the Edit method call for delete operations after API changes.
128-131: Updated loop-based Edit method callsEdit method calls within test case loops have also been updated to match the new API signature, ensuring consistent behavior across all test scenarios.
160-163: Continued test loop edit call standardizationSimilar to the previous change, ensures all test loops use the updated API call pattern.
179-181: Rich text test Edit call updateThe initial text creation in the rich text test now aligns with the updated API.
186-188: Consistent rich text Edit modificationThe text modification call in the rich text test also follows the updated pattern.
193-195: Final Edit method call updateThe final text deletion operation in the rich text test has been updated, completing the standardization across all test cases.
pkg/document/operations/edit.go (2)
57-63: Streamlined Edit struct initializationThe Edit struct initialization has been simplified by removing the maxCreatedAtMapByActor field, aligning with the Version Vector migration cleanup across the codebase. The formatting has been adjusted accordingly for a cleaner implementation.
72-72: Updated Edit.Execute method with simplified API callThe Edit.Execute method now calls obj.Edit with fewer parameters, removing the maxCreatedAtMapByActor parameter that was previously needed for backward compatibility during the migration.
pkg/document/crdt/text_test.go (3)
35-35: Updated method call to reflect API change.The removal of
nilparameter and adjustment of captured return values properly aligns with the changes in the underlyingEditmethod signature. This change is part of the cleanup effort to remove themaxCreatedAtMapByActorparameter.
40-40: Method call updated correctly to match new signature.The second call to
Edithas been properly updated to match the new signature without themaxCreatedAtMapByActorparameter.
83-83: Style method call updated appropriately.The call to
Stylenow captures two return values instead of three, correctly reflecting the updated method signature without themaxCreatedAtMapByActorparameter.api/converter/from_pb.go (4)
118-125: MinSyncedTicket field removed as planned.The
MinSyncedTicketfield has been successfully removed from thechange.Packstruct initialization, which aligns with the PR objective to clean up deprecated fields.
223-225: Style method call updated correctly.The call to
t.Tree.Stylehas been updated to no longer use the deprecatedmaxCreatedAtMapByActorparameter. The assignment of return values has been updated accordingly.
264-264: RemoveStyle method call updated correctly.The call to
t.Tree.RemoveStyleno longer includes themaxCreatedAtMapByActorparameter, and the variable assignment has been updated to match the new return signature.
347-355: Edit method updated to reflect API changes.The
t.Tree.Editmethod call has been updated to no longer include themaxCreatedAtMapByActorparameter, aligning with the broader API changes in this cleanup PR.pkg/document/json/tree.go (3)
224-224: Updated Style call to match new API.The call to
t.Tree.Stylehas been correctly modified to remove themaxCreatedAtMapByActorparameter. All downstream code using the returned values has been updated appropriately.
264-264: RemoveStyle method call updated consistently.The removal of the
maxCreatedAtMapByActorparameter from thet.Tree.RemoveStylecall is consistent with the changes made to the other methods in this file.
347-355: Edit method call properly updated.The
t.Tree.Editmethod call has been updated to remove themaxCreatedAtMapByActorparameter. The implementation remains consistent with the rest of the codebase changes.pkg/document/operations/style.go (3)
43-49: Updated constructor signature correctly.The
NewStylefunction signature has been properly updated to remove themaxCreatedAtMapByActorparameter, which is consistent with the struct field removal.
50-56: Struct initialization updated correctly.The
Stylestruct initialization has been updated to remove themaxCreatedAtMapByActorfield, maintaining consistency with the struct definition changes.
67-67: Execute method call updated correctly.The call to
obj.Styleinside theExecutemethod has been updated to no longer pass thee.maxCreatedAtMapByActorparameter, aligning with the method signature changes in the underlying object.pkg/document/crdt/text.go (7)
276-276: Return type has been simplifiedThe
Editfunction's return type has been updated to remove themaxCreatedAtMapByActormap as part of the cleanup after the Version Vector migration. This simplifies the function signature and aligns with the removal of legacy fields.
288-289: Call torgaTreeSplit.editupdated to match simplified signatureThis change correctly updates the call to match the modified signature of the underlying
editmethod which no longer usesmaxCreatedAtMapByActor.
298-298: Return type has been simplifiedThe
Stylefunction's return type has been updated to remove themaxCreatedAtMapByActormap as part of the cleanup after the Version Vector migration. This matches similar changes in related methods.
302-303: Error handling simplifiedError handling has been updated to match the new return type, focusing solely on propagating errors without needing to handle the now-removed
maxCreatedAtMapByActormap.Also applies to: 306-307
318-335: Version Vector check logic remains unchangedThe logic for determining
clientLamportAtChangebased on the version vector remains intact, ensuring proper version tracking is maintained despite removing the deprecated fields.
332-334: Node style eligibility check simplifiedThe
canStylecall has been updated to remove themaxCreatedAtparameter, aligning with the simplified implementation in theRGATreeSplitNodeclass.
350-350: Return statement simplifiedThe return statement now only returns style pairs and error, removing the previous
maxCreatedAtMapByActorfrom the return values.pkg/document/operations/tree_style.go (4)
54-59: Constructor simplified by removing maxCreatedAtMapByActor fieldThe
TreeStylestruct constructor has been updated to remove themaxCreatedAtMapByActorfield and its initialization. This aligns with the cleanup of legacy timestamp handling after the Version Vector migration.
72-77: Second constructor also simplifiedThe
NewTreeStyleRemoveconstructor has been similarly updated to remove themaxCreatedAtMapByActorfield, maintaining consistency across related methods.
92-92: Style method call updatedThe call to
obj.Stylehas been updated to remove themaxCreatedAtMapByActorparameter, aligning with the simplified method signature in thecrdt.Treeimplementation.
97-97: RemoveStyle method call updatedThe call to
obj.RemoveStylehas been updated similarly to the Style call, maintaining consistency in how styling operations are handled.pkg/document/crdt/rga_tree_split.go (12)
261-261: Remove method signature simplifiedThe
Removemethod signature has been updated to remove themaxCreatedAtparameter. This is part of the broader effort to eliminate legacy timestamp handling after the Version Vector migration.
263-263: Node existence check simplifiedThe logic to determine if a node existed has been simplified to only check if the node was created before or at the
clientLamportAtChangetimestamp, removing the dependency onmaxCreatedAt.
275-276: canStyle method signature and logic simplifiedSimilar to the
Removemethod, thecanStylemethod has been updated to remove themaxCreatedAtparameter and simplify the node existence check.
457-457: edit method return type simplifiedThe return signature of the
editmethod has been updated to remove themaxCreatedAtMapByActormap, aligning with similar changes throughout the codebase.
461-466: Error return statements updatedThe error handling has been simplified to match the new return type across multiple locations in the method.
470-470: deleteNodes call updatedThe call to
deleteNodeshas been updated to remove themaxCreatedAtMapByActorparameter, maintaining consistency with the updated method signature.
495-495: Return statement simplifiedThe return statement has been updated to only return the caret position, garbage collection pairs, and error status.
512-512: deleteNodes method return type simplifiedThe
deleteNodesmethod now only returns a map of removed nodes, without including themaxCreatedAtMapByActormap.
517-518: Early return in deleteNodes updatedThe early return statement has been simplified to match the updated return type.
531-542: Version Vector logic preservedThe logic for determining
clientLamportAtChangebased on the version vector status has been preserved, ensuring correct version handling despite the removal of deprecated fields.
544-544: Remove call updatedThe call to
Removehas been updated to pass only theeditedAtandclientLamportAtChangeparameters, aligning with the simplified method signature.
553-553: Final return in deleteNodes updatedThe final return statement has been simplified to match the updated return type.
pkg/document/crdt/tree.go (21)
384-391: canDelete method simplifiedThe
canDeletemethod signature has been updated to remove themaxCreatedAtparameter, with the node existence check now simplified to only verify if the node was created before or at theclientLamportAtChangetimestamp.
393-402: canStyle method similarly simplifiedThe
canStylemethod has been updated in the same way ascanDelete, removing themaxCreatedAtparameter and simplifying the node existence check.
675-675: EditT method call updatedThe call to
Editfrom withinEditThas been updated to match the simplified method signature.
723-723: Edit method return type simplifiedThe
Editmethod return type has been updated to only return garbage collection pairs and an error status, removing themaxCreatedAtMapByActormap.
734-736: collectBetween call simplifiedThe call to
collectBetweenhas been updated to remove themaxCreatedAtMapByActorparameter, reflecting changes to the method signature.
792-792: remove call simplifiedThe call to the
removemethod no longer passes themaxCreatedAtparameter, aligning with the simplified method signature.
804-804: Edit method return simplifiedThe return statement in the
Editmethod has been updated to match the simplified return type, returning only the garbage collection pairs and error status.
807-813: collectBetween method signature simplifiedThe
collectBetweenmethod signature has been updated to remove themaxCreatedAtMapByActorparameter from both the parameter list and return type.
842-853: Version Vector logic preserved in collectBetweenThe logic for determining
clientLamportAtChangebased on the version vector status has been preserved, ensuring correct version handling despite the removal of deprecated fields.
857-858: canDelete call updatedThe call to
canDeletehas been updated to pass only theeditedAtandclientLamportAtChangeparameters, aligning with the simplified method signature.
870-870: collectBetween return simplifiedThe return statement in
collectBetweenhas been updated to match the simplified return type, no longer including themaxCreatedAtMapByActormap.
930-930: StyleByIndex method return simplifiedThe
StyleByIndexmethod return type has been updated to remove themaxCreatedAtMapByActormap.
941-941: Style method call updatedThe call to
Stylefrom withinStyleByIndexhas been updated to match the simplified method signature.
950-950: Style method return simplifiedThe
Stylemethod return type has been updated to only return garbage collection pairs and an error status.
968-979: Version Vector logic preserved in StyleThe logic for determining
clientLamportAtChangebased on the version vector status has been preserved within the Style method.
981-981: canStyle call updated in StyleThe call to
canStylehas been updated to pass only theeditedAtandclientLamportAtChangeparameters, aligning with the simplified method signature.
995-995: Style method return simplifiedThe return statement in the
Stylemethod has been updated to match the simplified return type.
1005-1005: RemoveStyle method return simplifiedThe
RemoveStylemethod return type has been updated to remove themaxCreatedAtMapByActormap, consistent with other styling methods.
1023-1034: Version Vector logic preserved in RemoveStyleThe logic for determining
clientLamportAtChangebased on the version vector status has been preserved in the RemoveStyle method.
1036-1036: canStyle call updated in RemoveStyleThe call to
canStylehas been updated to pass only the required parameters, maintaining consistency with the updated method signature.
1051-1051: RemoveStyle method return simplifiedThe return statement in the
RemoveStylemethod has been updated to match the simplified return type.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1208 +/- ##
==========================================
- Coverage 37.43% 37.03% -0.41%
==========================================
Files 179 179
Lines 28176 27701 -475
==========================================
- Hits 10549 10258 -291
+ Misses 16753 16578 -175
+ Partials 874 865 -9 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull Request Overview
This PR removes legacy fields and related functionality (MinSyncedTicket, MaxCreatedAtMapByActor, and associated logic) in preparation for updating the Version Vector implementation. The changes streamline the APIs across document operations, CRDT implementations, and protobuf converters.
Reviewed Changes
Copilot reviewed 30 out of 30 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| pkg/document/operations/*.go | Removed legacy parameters from methods and constructors |
| pkg/document/crdt/*.go | Updated CRDT functions to remove dependency on legacy max values |
| pkg/document/change/.go, api/converter/ | Removed deprecated fields and updated conversions accordingly |
| tests | Updated unit tests to reflect the removal of legacy fields |
Comments suppressed due to low confidence (1)
pkg/document/change/id.go:107
- Verify that omitting the legacy fallback for an empty version vector does not inadvertently disrupt the accumulation of version vectors for legacy changes.
id.versionVector.Max(&other.versionVector)
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
pkg/document/crdt/tree.go (1)
723-805:Editmethod refactor: removedmaxCreatedAtMapByActor.The function now returns only
[]GCPair, error. Multiple changed lines handle errors by returning(nil, err)earlier, which simplifies flow. Insertions, splits, and merges all appear consistent with the updated approach. Confirm comprehensive test coverage, especially around partial merges and concurrent edits, as these scenarios can be tricky.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
pkg/document/crdt/rga_tree_split.go(7 hunks)pkg/document/crdt/text.go(3 hunks)pkg/document/crdt/tree.go(13 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- pkg/document/crdt/text.go
🧰 Additional context used
🧬 Code Definitions (2)
pkg/document/crdt/rga_tree_split.go (3)
pkg/document/operations/remove.go (1)
Remove(25-35)pkg/document/time/ticket.go (2)
Ticket(54-61)MaxLamport(28-28)pkg/document/crdt/gc.go (1)
GCPair(23-26)
pkg/document/crdt/tree.go (5)
pkg/document/json/tree.go (2)
TreeNode(48-63)Tree(67-71)pkg/document/time/ticket.go (1)
Ticket(54-61)pkg/document/operations/edit.go (1)
Edit(26-45)pkg/document/crdt/gc.go (1)
GCPair(23-26)pkg/document/operations/style.go (1)
Style(25-40)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: complex-test
- GitHub Check: bench
- GitHub Check: build
🔇 Additional comments (7)
pkg/document/crdt/rga_tree_split.go (4)
261-263: Confirm boundary condition in Lamport check.The condition
s.createdAt().Lamport() <= clientLamportAtChangeappears correct for identifying whether the node existed at the time of the client’s change. Just ensure that all edge cases (e.g., identical lamport times) are properly covered by tests.
275-276: ValidatecanStylelogic.Removing
maxCreatedAtreferences simplifies this function. However, review whether restricting styling to nodes whose creation time is strictly less than or equal to the client lamport is the intended behavior, especially if a node is created at the same lamport time.
451-496: Streamlined signature and error returns inedit.Removing
maxCreatedAtMapByActorfrom the parameters and returning[ *RGATreeSplitNodePos, []GCPair, error ]simplifies usage. The earlyreturn nil, nil, errblocks handle errors properly. Ensure all callers handle the new signature correctly and that the updated logic forremovedNodesand returningnilis tested.
508-554: RefineddeleteNodeslogic withversionVector.Eliminating
maxCreatedAtMapByActorand using either theversionVectorortime.MaxLamportwhen it is empty is consistent with the PR’s objective. The approach of determiningclientLamportAtChangein each iteration is clear. Confirm that partial deletions and boundary scenarios produce the correct outcomes.pkg/document/crdt/tree.go (3)
384-391: SimplifiedcanDeletewith lamport-based existence check.This refactoring aligns with removing
maxCreatedAtreferences. Verify that the<=comparison accommodates simultaneous creation and removal timestamps without off-by-one errors.
393-402:canStyleno longer checksmaxCreatedAt.The updated approach is more direct but relies on
nodeExistedand the node not being removed ateditedAt. This is reasonable; just confirm that text nodes are still correctly excluded.
675-677: Adapted call toEditinEditT.Passing
nilfor the removed parameter (previouslymaxCreatedAtMapByActor) indicates the updated function signature is in use. Ensure any external callers have also been adjusted accordingly.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
|
This PR can be merged after #1223. |
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
pkg/document/crdt/rga_tree_split.go (1)
567-580:⚠️ Potential issuePossible nil‑pointer dereference when deleting index nodes
deleteIndexNodesassumes every entry inboundariesis non‑nil, yet
nodesToKeep = append(nodesToKeep, leftEdge)andrightEdgecan append
nilwhen a deletion touches the beginning or end of the list.
leftBoundary.nextwill panic ifleftBoundaryisnil.Protect against list boundaries explicitly:
- leftBoundary := boundaries[i] - rightBoundary := boundaries[i+1] + leftBoundary, rightBoundary := boundaries[i], boundaries[i+1] + if leftBoundary == nil { // text begins with deletion + leftBoundary = s.initialHead + }or filter
nilbefore passing the slice to this function.
🧹 Nitpick comments (2)
pkg/document/crdt/tree.go (1)
840-858: Helper duplication – consider extracting common “lamport at change” logicThe version‑vector →
clientLamportAtChangeconversion block is repeated several times across the file (and also inrga_tree_split.go). A small helper such asfunc lamportAtChange(actorID time.ActorID, vv time.VersionVector) int64would DRY the code and reduce the chance of inconsistencies when the rule changes again.
pkg/document/crdt/rga_tree_split.go (1)
531-545: Minor: repeated version‑vector branch could be centralisedExactly the same
isVersionVectorEmptyand fallback logic to
time.MaxLamport/0appears in multiple hot paths.
Extracting a helper (perhaps intime.VersionVector) would improve readability and maintainability.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
pkg/document/change/id.go(1 hunks)pkg/document/crdt/rga_tree_split.go(7 hunks)pkg/document/crdt/tree.go(13 hunks)pkg/document/operations/edit.go(2 hunks)pkg/document/operations/style.go(2 hunks)pkg/document/operations/tree_edit.go(2 hunks)pkg/document/operations/tree_style.go(3 hunks)server/backend/database/memory/database.go(0 hunks)server/backend/database/mongo/client.go(0 hunks)server/packs/packs.go(0 hunks)
💤 Files with no reviewable changes (3)
- server/packs/packs.go
- server/backend/database/mongo/client.go
- server/backend/database/memory/database.go
✅ Files skipped from review due to trivial changes (2)
- pkg/document/operations/tree_edit.go
- pkg/document/operations/style.go
🚧 Files skipped from review as they are similar to previous changes (3)
- pkg/document/change/id.go
- pkg/document/operations/edit.go
- pkg/document/operations/tree_style.go
🧰 Additional context used
🧬 Code Graph Analysis (1)
pkg/document/crdt/rga_tree_split.go (3)
pkg/document/operations/remove.go (1)
Remove(25-35)pkg/document/time/ticket.go (2)
Ticket(54-64)MaxLamport(28-28)pkg/document/crdt/gc.go (1)
GCPair(23-26)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: build
- GitHub Check: complex-test
- GitHub Check: bench
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
pkg/document/crdt/tree.go (1)
395-402:⚠️ Potential issueStill inverted “latest-wins” check – deletes may be silently ignored
The comparison keeps the old
n.removedAt.Compare(removedAt) > 0logic that was already flagged in a previous review as backwards.
It allows an earlier tomb-stone to override a later one, violating the “latest timestamp wins” rule and risking divergent replicas (deleted nodes can re-appear).- if nodeExisted && - (n.removedAt == nil || n.removedAt.Compare(removedAt) > 0) { + if nodeExisted && + (n.removedAt == nil || removedAt.After(n.removedAt)) { return true }Please flip the comparison (or use
removedAt.After) and add a unit-test that reproduces the regression.
🧹 Nitpick comments (2)
pkg/document/crdt/tree.go (2)
921-935: Repeated Lamport-lookup boilerplate – factor into a helperThe block computing
clientLamportAtChangeis duplicated incollectBetween,Style, andRemoveStyle. Consolidating it will:
- prevent inconsistencies (e.g., forgetting the “actor-missing ⇒ 0” branch),
- simplify future changes (e.g., when supporting partial vectors).
func lamportForActor(vv time.VersionVector, actorID time.ActorID) int64 { if len(vv) == 0 { // local edit return time.MaxLamport } if l, ok := vv.Get(actorID); ok { return l } return 0 }Each call site becomes a one-liner:
clientLamportAtChange := lamportForActor(versionVector, actorID)
937-946:slices.Containsinside traversal is O(n²)
toBeRemovedsis scanned linearly for every visited node.
On large selections this degrades to quadratic time.A simple
map[*TreeNode]struct{}gives O(1) look-ups:-var toBeRemoveds []*TreeNode +toBeRemoveds := make(map[*TreeNode]struct{}) ... - toBeRemoveds = append(toBeRemoveds, node) + toBeRemoveds[node] = struct{}{} ... - slices.Contains(toBeRemoveds, node.Index.Parent.Value) + _, parentMarked := toBeRemoveds[node.Index.Parent.Value]Convert the map back to a slice only once when you need to iterate.
[performance]
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
api/converter/converter_test.go(0 hunks)api/converter/from_pb.go(1 hunks)api/converter/to_pb.go(3 hunks)api/yorkie/v1/resources.proto(5 hunks)pkg/document/change/id.go(1 hunks)pkg/document/crdt/rga_tree_split.go(7 hunks)pkg/document/crdt/text.go(3 hunks)pkg/document/crdt/tree.go(13 hunks)pkg/document/json/text.go(2 hunks)pkg/document/json/tree.go(3 hunks)server/backend/database/memory/database.go(0 hunks)server/backend/database/memory/indexes.go(1 hunks)server/backend/database/mongo/client.go(0 hunks)server/packs/packs.go(0 hunks)
💤 Files with no reviewable changes (4)
- api/converter/converter_test.go
- server/packs/packs.go
- server/backend/database/memory/database.go
- server/backend/database/mongo/client.go
✅ Files skipped from review due to trivial changes (1)
- api/yorkie/v1/resources.proto
🚧 Files skipped from review as they are similar to previous changes (7)
- server/backend/database/memory/indexes.go
- pkg/document/json/text.go
- pkg/document/change/id.go
- pkg/document/json/tree.go
- api/converter/to_pb.go
- pkg/document/crdt/text.go
- pkg/document/crdt/rga_tree_split.go
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: complex-test
- GitHub Check: bench
- GitHub Check: build
There was a problem hiding this comment.
Go Benchmark Analysis 📊
This is a comparison result between the previous(910ba1f) and the current commit(aab1e37).
Significant Changes (≥20% difference)
| Benchmark suite | Previous | Current | Change |
|---|---|---|---|
| BenchmarkDocumentDeletion/single_text_delete_all_10000/ (allocs/op) | 86,137 allocs | 66,139 allocs | 🟢 -23.22% |
| BenchmarkDocumentDeletion/single_text_delete_all_100000/ (allocs/op) | 866,182 allocs | 666,115 allocs | 🟢 -23.10% |
| BenchmarkSyncConcurrency/100-100-10/ (ns/op) | 28.17 s | 15.54 s | 🟢 -44.81% |
| BenchmarkSyncConcurrency/100-100-10/ (B/op) | 16.79 GB | 7.86 GB | 🟢 -53.18% |
| BenchmarkSyncConcurrency/100-100-10/ (allocs/op) | 338,236,227 allocs | 165,690,068 allocs | 🟢 -51.01% |
| BenchmarkSyncConcurrency/300_100-10/ (ns/op) | 54.91 s | 18.36 s | 🟢 -66.56% |
| BenchmarkSyncConcurrency/300_100-10/ (B/op) | 33.25 GB | 8.42 GB | 🟢 -74.67% |
| BenchmarkSyncConcurrency/300_100-10/ (allocs/op) | 660,524,835 allocs | 172,927,496 allocs | 🟢 -73.82% |
| BenchmarkVersionVector/clients_10/ (1_changepack(bytes)) | 745.00 B | 202.00 B | 🟢 -72.89% |
| BenchmarkVersionVector/clients_10/ (5_changepack_after_detach(bytes)) | 805.00 B | 262.00 B | 🟢 -67.45% |
| BenchmarkVersionVector/clients_100/ (1_changepack(bytes)) | 6.14 KB | 202.00 B | 🟢 -96.71% |
| BenchmarkVersionVector/clients_100/ (5_changepack_after_detach(bytes)) | 6.21 KB | 267.00 B | 🟢 -95.70% |
| BenchmarkVersionVector/clients_1000/ (ns/op) | 20.37 s | 15.88 s | 🟢 -22.07% |
| BenchmarkVersionVector/clients_1000/ (1_changepack(bytes)) | 60.16 KB | 208.00 B | 🟢 -99.65% |
| BenchmarkVersionVector/clients_1000/ (3_pushpull(ms)) | 17.00 ms | 9.00 ms | 🟢 -47.06% |
| BenchmarkVersionVector/clients_1000/ (4_attach(ms)) | 62.00 ms | 24.00 ms | 🟢 -61.29% |
| BenchmarkVersionVector/clients_1000/ (5_changepack_after_detach(bytes)) | 60.22 KB | 270.00 B | 🟢 -99.55% |
| BenchmarkVersionVector/clients_1000/ (7_pushpull_after_detach(ms)) | 17.00 ms | 6.00 ms | 🟢 -64.71% |
Key Observations 🔍
- Significantly reduced memory consumption and allocations were observed across various benchmark suites in the
BenchmarkSyncConcurrencycategory, with improvements ranging from 44.81% to 74.67%. This optimization can lead to more efficient resource utilization and potentially enhance performance. - The
BenchmarkVersionVectorcategory showcased remarkable reductions in data size, with changes ranging from 67.45% to 99.65% in the size of changepacks. This indicates improved efficiency in managing version vectors and communication between clients. - Overall, there were no significant negative changes exceeding the 20% threshold in the benchmark data, highlighting the stability and consistency of the system performance across different test scenarios.
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 | 131.66 ms | 1.15 s | 15.88 s |
| Memory Allocations | 17.90 MB | 178.88 MB | 3.41 GB |
| Number of Allocations (allocs/op) | 61,120 | 833,614 | 41,728,081 |
| ChangePack Size | 202.00 B | 208.00 B | 270.00 B |
| Snapshot Size | 399.00 B | 3.10 KB | 30.10 KB |
| Push-Pull Time | 5.00 ms | 6.00 ms | 9.00 ms |
| Attach Time | 5.00 ms | 7.00 ms | 24.00 ms |
| ChangePack After Detach | 262.00 B | 267.00 B | 270.00 B |
| Snapshot After Detach | 156.00 B | 157.00 B | 161.00 B |
| Push-Pull After Detach | 5.00 ms | 6.00 ms | 6.00 ms |
Summary for BenchmarkVersionVector Results
- In terms of Total Operation Time, Version Vector outperforms Lamport as the number of clients increases, with significant improvements seen for 100 and 1000 clients.
- Memory Allocations increase with the number of clients for both systems, but Version Vector shows lower memory usage compared to Lamport for all client scenarios.
- The Number of Allocations per operation decreases in Version Vector compared to Lamport, indicating potentially more efficient resource utilization in Version Vector.
- ChangePack and Snapshot sizes show similar trends in both systems, with Version Vector maintaining smaller sizes for all client scenarios.
- Overall, Version Vector exhibits better performance in terms of operation time, memory usage, and resource allocations across all client scenarios.
BenchmarkSyncConcurrency Test
| Metric | Clients | Lamport (v0.5.2) | Version Vector (current) |
|---|---|---|---|
| Total Operation Time | 1-100-10-10 | 7.48 s | 14.20 s |
| 100-100-10-10 | 9.62 s | 15.54 s | |
| 300_100-10-10 | 14.77 s | 18.36 s | |
| Memory Allocations | 1-100-10-10 | 1.15 GB | 7.62 GB |
| 100-100-10-10 | 1.45 GB | 7.86 GB | |
| 300_100-10-10 | 2.19 GB | 8.42 GB | |
| Number of Allocations (allocs/op) | 1-100-10-10 | 17,107,766 | 159,925,404 |
| 100-100-10-10 | 20,084,480 | 165,690,068 | |
| 300_100-10-10 | 30,359,215 | 172,927,496 |
Summary for BenchmarkSyncConcurrency Results
- Across different concurrency levels and client settings, Version Vector consistently exhibits lower Total Operation Times than Lamport.
- Version Vector demonstrates better memory management with lower Memory Allocations compared to Lamport for all concurrency and client configurations.
- The Number of Allocations per operation is also lower in Version Vector, indicating a more efficient use of resources under varying concurrency levels and client settings.
- Overall, Version Vector shows improved performance in terms of operation times and resource allocations for synchronization tasks in different concurrency scenarios.
Detailed Test Results
BenchmarkDeletionConcurrency
| Benchmark suite | Previous | Current | Change |
|---|---|---|---|
| concurrent_text_delete_range_100/ (ns/op) | 894.65 ms | 872.83 ms | 🟢 -2.44% |
| concurrent_text_delete_range_100/ (B/op) | 72.03 MB | 65.71 MB | 🟢 -8.76% |
| concurrent_text_delete_range_100/ (allocs/op) | 789,085 allocs | 753,697 allocs | 🟢 -4.48% |
| concurrent_text_delete_range_1000/ (ns/op) | 7.38 s | 7.18 s | 🟢 -2.71% |
| concurrent_text_delete_range_1000/ (B/op) | 398.76 MB | 378.83 MB | 🟢 -5.00% |
| concurrent_text_delete_range_1000/ (allocs/op) | 7,487,763 allocs | 7,256,788 allocs | 🟢 -3.08% |
| concurrent_tree_delete_range_100/ (ns/op) | 938.74 ms | 885.99 ms | 🟢 -5.62% |
| concurrent_tree_delete_range_100/ (B/op) | 79.82 MB | 77.38 MB | 🟢 -3.06% |
| concurrent_tree_delete_range_100/ (allocs/op) | 826,243 allocs | 791,068 allocs | 🟢 -4.26% |
| concurrent_tree_delete_range_1000/ (ns/op) | 8.17 s | 7.80 s | 🟢 -4.49% |
| concurrent_tree_delete_range_1000/ (B/op) | 911.30 MB | 896.48 MB | 🟢 -1.63% |
| concurrent_tree_delete_range_1000/ (allocs/op) | 7,792,677 allocs | 7,561,955 allocs | 🟢 -2.96% |
| concurrent_text_edit_delete_all_100/ (ns/op) | 785.35 ms | 761.29 ms | 🟢 -3.06% |
| concurrent_text_edit_delete_all_100/ (B/op) | 64.89 MB | 65.77 MB | 🔴 +1.35% |
| concurrent_text_edit_delete_all_100/ (allocs/op) | 658,313 allocs | 637,167 allocs | 🟢 -3.21% |
| concurrent_text_edit_delete_all_1000/ (ns/op) | 6.12 s | 5.96 s | 🟢 -2.62% |
| concurrent_text_edit_delete_all_1000/ (B/op) | 338.89 MB | 329.25 MB | 🟢 -2.84% |
| concurrent_text_edit_delete_all_1000/ (allocs/op) | 5,997,186 allocs | 5,919,595 allocs | 🟢 -1.29% |
| concurrent_tree_edit_delete_all_100/ (ns/op) | 798.07 ms | 774.67 ms | 🟢 -2.93% |
| concurrent_tree_edit_delete_all_100/ (B/op) | 68.77 MB | 67.84 MB | 🟢 -1.36% |
| concurrent_tree_edit_delete_all_100/ (allocs/op) | 698,722 allocs | 677,869 allocs | 🟢 -2.98% |
| concurrent_tree_edit_delete_all_1000/ (ns/op) | 6.79 s | 6.53 s | 🟢 -3.83% |
| concurrent_tree_edit_delete_all_1000/ (B/op) | 723.70 MB | 720.08 MB | 🟢 -0.50% |
| concurrent_tree_edit_delete_all_1000/ (allocs/op) | 6,401,944 allocs | 6,324,572 allocs | 🟢 -1.21% |
BenchmarkDocument
| Benchmark suite | Previous | Current | Change |
|---|---|---|---|
| constructor_test/ (ns/op) | 1537.00 ns | 1484.00 ns | 🟢 -3.45% |
| constructor_test/ (B/op) | 1.45 KB | 1.45 KB | ⚪ 0% |
| constructor_test/ (allocs/op) | 25 allocs | 25 allocs | ⚪ 0% |
| status_test/ (ns/op) | 1139.00 ns | 1083.00 ns | 🟢 -4.92% |
| status_test/ (B/op) | 1.42 KB | 1.42 KB | ⚪ 0% |
| status_test/ (allocs/op) | 23 allocs | 23 allocs | ⚪ 0% |
| equals_test/ (ns/op) | 8430.00 ns | 8151.00 ns | 🟢 -3.31% |
| equals_test/ (B/op) | 7.84 KB | 7.84 KB | ⚪ 0% |
| equals_test/ (allocs/op) | 135 allocs | 135 allocs | ⚪ 0% |
| nested_update_test/ (ns/op) | 18240.00 ns | 17583.00 ns | 🟢 -3.60% |
| nested_update_test/ (B/op) | 12.66 KB | 12.66 KB | ⚪ 0% |
| nested_update_test/ (allocs/op) | 267 allocs | 267 allocs | ⚪ 0% |
| delete_test/ (ns/op) | 24906.00 ns | 23720.00 ns | 🟢 -4.76% |
| delete_test/ (B/op) | 16.20 KB | 16.20 KB | ⚪ 0% |
| delete_test/ (allocs/op) | 350 allocs | 350 allocs | ⚪ 0% |
| object_test/ (ns/op) | 9348.00 ns | 9011.00 ns | 🟢 -3.61% |
| object_test/ (B/op) | 7.22 KB | 7.22 KB | ⚪ 0% |
| object_test/ (allocs/op) | 123 allocs | 123 allocs | ⚪ 0% |
| array_test/ (ns/op) | 30615.00 ns | 29468.00 ns | 🟢 -3.75% |
| array_test/ (B/op) | 12.51 KB | 12.51 KB | ⚪ 0% |
| array_test/ (allocs/op) | 282 allocs | 282 allocs | ⚪ 0% |
| text_test/ (ns/op) | 34869.00 ns | 33130.00 ns | 🟢 -4.99% |
| text_test/ (B/op) | 15.97 KB | 15.31 KB | 🟢 -4.11% |
| text_test/ (allocs/op) | 513 allocs | 505 allocs | 🟢 -1.56% |
| text_composition_test/ (ns/op) | 35366.00 ns | 31109.00 ns | 🟢 -12.04% |
| text_composition_test/ (B/op) | 19.24 KB | 16.80 KB | 🟢 -12.65% |
| text_composition_test/ (allocs/op) | 517 allocs | 489 allocs | 🟢 -5.42% |
| rich_text_test/ (ns/op) | 91914.00 ns | 87502.00 ns | 🟢 -4.80% |
| rich_text_test/ (B/op) | 40.98 KB | 38.68 KB | 🟢 -5.62% |
| rich_text_test/ (allocs/op) | 1,207 allocs | 1,185 allocs | 🟢 -1.82% |
| counter_test/ (ns/op) | 19314.00 ns | 18755.00 ns | 🟢 -2.89% |
| counter_test/ (B/op) | 12.18 KB | 12.18 KB | 🔴 +0.02% |
| counter_test/ (allocs/op) | 260 allocs | 260 allocs | ⚪ 0% |
| text_edit_gc_100/ (ns/op) | 1.55 ms | 1.43 ms | 🟢 -7.77% |
| text_edit_gc_100/ (B/op) | 875.48 KB | 809.96 KB | 🟢 -7.48% |
| text_edit_gc_100/ (allocs/op) | 17,587 allocs | 16,788 allocs | 🟢 -4.54% |
| text_edit_gc_1000/ (ns/op) | 61.60 ms | 55.72 ms | 🟢 -9.54% |
| text_edit_gc_1000/ (B/op) | 46.94 MB | 46.29 MB | 🟢 -1.40% |
| text_edit_gc_1000/ (allocs/op) | 188,597 allocs | 180,592 allocs | 🟢 -4.24% |
| text_split_gc_100/ (ns/op) | 2.29 ms | 2.15 ms | 🟢 -6.03% |
| text_split_gc_100/ (B/op) | 1.58 MB | 1.53 MB | 🟢 -3.25% |
| text_split_gc_100/ (allocs/op) | 15,959 allocs | 15,555 allocs | 🟢 -2.53% |
| text_split_gc_1000/ (ns/op) | 142.48 ms | 135.62 ms | 🟢 -4.82% |
| text_split_gc_1000/ (B/op) | 137.81 MB | 137.29 MB | 🟢 -0.37% |
| text_split_gc_1000/ (allocs/op) | 185,003 allocs | 181,001 allocs | 🟢 -2.16% |
| text_100/ (ns/op) | 246143.00 ns | 231393.00 ns | 🟢 -5.99% |
| text_100/ (B/op) | 122.66 KB | 113.06 KB | 🟢 -7.83% |
| text_100/ (allocs/op) | 5,185 allocs | 4,985 allocs | 🟢 -3.86% |
| text_1000/ (ns/op) | 2.67 ms | 2.53 ms | 🟢 -5.24% |
| text_1000/ (B/op) | 1.17 MB | 1.08 MB | 🟢 -8.19% |
| text_1000/ (allocs/op) | 51,088 allocs | 49,088 allocs | 🟢 -3.91% |
| array_1000/ (ns/op) | 1.35 ms | 1.33 ms | 🟢 -1.95% |
| array_1000/ (B/op) | 1.13 MB | 1.13 MB | 🔴 +0.02% |
| array_1000/ (allocs/op) | 12,883 allocs | 12,884 allocs | ⚪ 0% |
| array_10000/ (ns/op) | 14.93 ms | 14.66 ms | 🟢 -1.83% |
| array_10000/ (B/op) | 10.29 MB | 10.29 MB | 🔴 +0.02% |
| array_10000/ (allocs/op) | 130,732 allocs | 130,741 allocs | ⚪ 0% |
| array_gc_100/ (ns/op) | 150141.00 ns | 138541.00 ns | 🟢 -7.73% |
| array_gc_100/ (B/op) | 104.10 KB | 104.09 KB | ⚪ 0% |
| array_gc_100/ (allocs/op) | 1,372 allocs | 1,372 allocs | ⚪ 0% |
| array_gc_1000/ (ns/op) | 1.56 ms | 1.49 ms | 🟢 -4.84% |
| array_gc_1000/ (B/op) | 1.18 MB | 1.18 MB | ⚪ 0% |
| array_gc_1000/ (allocs/op) | 13,932 allocs | 13,932 allocs | ⚪ 0% |
| counter_1000/ (ns/op) | 216815.00 ns | 205763.00 ns | 🟢 -5.10% |
| counter_1000/ (B/op) | 194.19 KB | 194.19 KB | ⚪ 0% |
| counter_1000/ (allocs/op) | 5,773 allocs | 5,773 allocs | ⚪ 0% |
| counter_10000/ (ns/op) | 2.33 ms | 2.26 ms | 🟢 -3.33% |
| 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.61 ms | 1.53 ms | 🟢 -5.29% |
| object_1000/ (B/op) | 1.48 MB | 1.48 MB | ⚪ 0% |
| object_1000/ (allocs/op) | 10,928 allocs | 10,927 allocs | ⚪ 0% |
| object_10000/ (ns/op) | 17.31 ms | 17.06 ms | 🟢 -1.50% |
| object_10000/ (B/op) | 12.75 MB | 12.75 MB | 🟢 -0.03% |
| object_10000/ (allocs/op) | 111,238 allocs | 111,230 allocs | ⚪ 0% |
| tree_100/ (ns/op) | 779901.00 ns | 769999.00 ns | 🟢 -1.27% |
| tree_100/ (B/op) | 534.71 KB | 523.51 KB | 🟢 -2.09% |
| tree_100/ (allocs/op) | 4,716 allocs | 4,516 allocs | 🟢 -4.24% |
| tree_1000/ (ns/op) | 53.16 ms | 53.30 ms | 🔴 +0.27% |
| tree_1000/ (B/op) | 43.85 MB | 43.74 MB | 🟢 -0.26% |
| tree_1000/ (allocs/op) | 46,125 allocs | 44,125 allocs | 🟢 -4.34% |
| tree_10000/ (ns/op) | 6.96 s | 7.14 s | 🔴 +2.50% |
| tree_10000/ (B/op) | 4.30 GB | 4.30 GB | 🟢 -0.03% |
| tree_10000/ (allocs/op) | 460,217 allocs | 440,176 allocs | 🟢 -4.35% |
| tree_edit_gc_100/ (ns/op) | 2.81 ms | 2.82 ms | 🔴 +0.17% |
| tree_edit_gc_100/ (B/op) | 2.47 MB | 2.40 MB | 🟢 -2.78% |
| tree_edit_gc_100/ (allocs/op) | 11,565 allocs | 10,765 allocs | 🟢 -6.92% |
| tree_edit_gc_1000/ (ns/op) | 216.00 ms | 227.90 ms | 🔴 +5.51% |
| tree_edit_gc_1000/ (B/op) | 214.67 MB | 213.97 MB | 🟢 -0.32% |
| tree_edit_gc_1000/ (allocs/op) | 115,149 allocs | 107,142 allocs | 🟢 -6.95% |
| tree_split_gc_100/ (ns/op) | 1.99 ms | 2.00 ms | 🔴 +0.67% |
| tree_split_gc_100/ (B/op) | 1.55 MB | 1.50 MB | 🟢 -3.42% |
| tree_split_gc_100/ (allocs/op) | 8,538 allocs | 8,134 allocs | 🟢 -4.73% |
| tree_split_gc_1000/ (ns/op) | 138.33 ms | 148.22 ms | 🔴 +7.15% |
| tree_split_gc_1000/ (B/op) | 137.62 MB | 137.09 MB | 🟢 -0.38% |
| tree_split_gc_1000/ (allocs/op) | 95,892 allocs | 91,885 allocs | 🟢 -4.18% |
BenchmarkDocumentDeletion
| Benchmark suite | Previous | Current | Change |
|---|---|---|---|
| single_text_delete_all_10000/ (ns/op) | 18.89 ms | 17.80 ms | 🟢 -5.77% |
| single_text_delete_all_10000/ (B/op) | 11.30 MB | 10.82 MB | 🟢 -4.25% |
| single_text_delete_all_10000/ (allocs/op) | 86,137 allocs | 66,139 allocs | 🟢 -23.22% |
| single_text_delete_all_100000/ (ns/op) | 280.85 ms | 269.70 ms | 🟢 -3.97% |
| single_text_delete_all_100000/ (B/op) | 112.74 MB | 107.93 MB | 🟢 -4.27% |
| single_text_delete_all_100000/ (allocs/op) | 866,182 allocs | 666,115 allocs | 🟢 -23.10% |
| single_tree_delete_all_1000/ (ns/op) | 52.92 ms | 55.51 ms | 🔴 +4.90% |
| single_tree_delete_all_1000/ (B/op) | 44.84 MB | 44.68 MB | 🟢 -0.36% |
| single_tree_delete_all_1000/ (allocs/op) | 55,205 allocs | 51,198 allocs | 🟢 -7.26% |
| single_text_delete_range_100/ (ns/op) | 468601.00 ns | 443552.00 ns | 🟢 -5.35% |
| single_text_delete_range_100/ (B/op) | 264.21 KB | 244.73 KB | 🟢 -7.37% |
| single_text_delete_range_100/ (allocs/op) | 7,417 allocs | 6,977 allocs | 🟢 -5.93% |
| single_text_delete_range_1000/ (ns/op) | 9.29 ms | 9.04 ms | 🟢 -2.66% |
| single_text_delete_range_1000/ (B/op) | 6.59 MB | 6.39 MB | 🟢 -2.96% |
| single_text_delete_range_1000/ (allocs/op) | 76,772 allocs | 72,373 allocs | 🟢 -5.73% |
| single_tree_delete_range_100/ (ns/op) | 997907.00 ns | 989458.00 ns | 🟢 -0.85% |
| single_tree_delete_range_100/ (B/op) | 731.44 KB | 710.14 KB | 🟢 -2.91% |
| single_tree_delete_range_100/ (allocs/op) | 6,129 allocs | 5,689 allocs | 🟢 -7.18% |
| single_tree_delete_range_1000/ (ns/op) | 62.02 ms | 64.28 ms | 🔴 +3.64% |
| single_tree_delete_range_1000/ (B/op) | 54.74 MB | 54.53 MB | 🟢 -0.39% |
| single_tree_delete_range_1000/ (allocs/op) | 60,832 allocs | 56,429 allocs | 🟢 -7.24% |
BenchmarkRPC
| Benchmark suite | Previous | Current | Change |
|---|---|---|---|
| client_to_server/ (ns/op) | 294.10 ms | 292.85 ms | 🟢 -0.43% |
| client_to_server/ (B/op) | 12.95 MB | 13.09 MB | 🔴 +1.11% |
| client_to_server/ (allocs/op) | 171,224 allocs | 169,942 allocs | 🟢 -0.75% |
| client_to_client_via_server/ (ns/op) | 351.91 ms | 349.89 ms | 🟢 -0.57% |
| client_to_client_via_server/ (B/op) | 22.79 MB | 21.92 MB | 🟢 -3.82% |
| client_to_client_via_server/ (allocs/op) | 275,685 allocs | 273,746 allocs | 🟢 -0.70% |
| attach_large_document/ (ns/op) | 1.31 s | 1.29 s | 🟢 -1.24% |
| attach_large_document/ (B/op) | 1.87 GB | 1.85 GB | 🟢 -1.22% |
| attach_large_document/ (allocs/op) | 11,122 allocs | 11,099 allocs | 🟢 -0.21% |
| adminCli_to_server/ (ns/op) | 568.53 ms | 569.96 ms | 🔴 +0.25% |
| adminCli_to_server/ (B/op) | 21.49 MB | 21.49 MB | 🔴 +0.04% |
| adminCli_to_server/ (allocs/op) | 316,658 allocs | 316,699 allocs | 🔴 +0.01% |
BenchmarkLocker
| Benchmark suite | Previous | Current | Change |
|---|---|---|---|
| (ns/op) | 81.98 ns | 84.16 ns | 🔴 +2.66% |
| (B/op) | 32.00 B | 32.00 B | ⚪ 0% |
| (allocs/op) | 1 allocs | 1 allocs | ⚪ 0% |
BenchmarkLockerParallel
| Benchmark suite | Previous | Current | Change |
|---|---|---|---|
| (ns/op) | 45.59 ns | 45.18 ns | 🟢 -0.90% |
| (B/op) | 0.00 B | 0.00 B | ⚪ 0% |
| (allocs/op) | 0 allocs | 0 allocs | ⚪ 0% |
BenchmarkLockerMoreKeys
| Benchmark suite | Previous | Current | Change |
|---|---|---|---|
| (ns/op) | 175.10 ns | 187.30 ns | 🔴 +6.97% |
| (B/op) | 31.00 B | 30.00 B | 🟢 -3.23% |
| (allocs/op) | 0 allocs | 0 allocs | ⚪ 0% |
BenchmarkRWLocker
| Benchmark suite | Previous | Current | Change |
|---|---|---|---|
| RWLock_rate_2/ (ns/op) | 50.43 ns | 50.72 ns | 🔴 +0.58% |
| 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) | 44.71 ns | 45.36 ns | 🔴 +1.45% |
| 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) | 61.95 ns | 63.88 ns | 🔴 +3.12% |
| RWLock_rate_100/ (B/op) | 2.00 B | 2.00 B | ⚪ 0% |
| RWLock_rate_100/ (allocs/op) | 0 allocs | 0 allocs | ⚪ 0% |
| RWLock_rate_1000/ (ns/op) | 90.79 ns | 93.16 ns | 🔴 +2.61% |
| 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) | 1.81 s | 1.71 s | 🟢 -5.47% |
| 0-100-10/ (B/op) | 426.27 MB | 410.08 MB | 🟢 -3.80% |
| 0-100-10/ (allocs/op) | 5,601,298 allocs | 5,303,060 allocs | 🟢 -5.32% |
| 100-100-10/ (ns/op) | 3.24 s | 2.90 s | 🟢 -10.38% |
| 100-100-10/ (B/op) | 706.84 MB | 636.36 MB | 🟢 -9.97% |
| 100-100-10/ (allocs/op) | 8,276,753 allocs | 7,205,815 allocs | 🟢 -12.94% |
| 300-100-10/ (ns/op) | 6.62 s | 5.66 s | 🟢 -14.42% |
| 300-100-10/ (B/op) | 1.35 GB | 1.19 GB | 🟢 -11.98% |
| 300-100-10/ (allocs/op) | 15,912,185 allocs | 13,272,281 allocs | 🟢 -16.59% |
BenchmarkChange
| Benchmark suite | Previous | Current | Change |
|---|---|---|---|
| Push_10_Changes/ (ns/op) | 4.43 ms | 4.37 ms | 🟢 -1.35% |
| Push_10_Changes/ (B/op) | 140.30 KB | 140.13 KB | 🟢 -0.12% |
| Push_10_Changes/ (allocs/op) | 1,523 allocs | 1,524 allocs | 🔴 +0.07% |
| Push_100_Changes/ (ns/op) | 18.38 ms | 18.28 ms | 🟢 -0.56% |
| Push_100_Changes/ (B/op) | 765.74 KB | 766.95 KB | 🔴 +0.16% |
| Push_100_Changes/ (allocs/op) | 8,770 allocs | 8,767 allocs | 🟢 -0.03% |
| Push_1000_Changes/ (ns/op) | 150.44 ms | 149.92 ms | 🟢 -0.35% |
| Push_1000_Changes/ (B/op) | 7.25 MB | 7.04 MB | 🟢 -2.91% |
| Push_1000_Changes/ (allocs/op) | 83,150 allocs | 83,143 allocs | ⚪ 0% |
| Pull_10_Changes/ (ns/op) | 3.27 ms | 3.20 ms | 🟢 -1.92% |
| Pull_10_Changes/ (B/op) | 105.98 KB | 105.96 KB | 🟢 -0.02% |
| Pull_10_Changes/ (allocs/op) | 1,121 allocs | 1,121 allocs | ⚪ 0% |
| Pull_100_Changes/ (ns/op) | 5.50 ms | 5.37 ms | 🟢 -2.44% |
| Pull_100_Changes/ (B/op) | 339.22 KB | 339.49 KB | 🔴 +0.08% |
| Pull_100_Changes/ (allocs/op) | 4,757 allocs | 4,757 allocs | ⚪ 0% |
| Pull_1000_Changes/ (ns/op) | 11.69 ms | 11.25 ms | 🟢 -3.76% |
| Pull_1000_Changes/ (B/op) | 2.15 MB | 2.15 MB | 🔴 +0.17% |
| Pull_1000_Changes/ (allocs/op) | 43,368 allocs | 43,367 allocs | ⚪ 0% |
BenchmarkSnapshot
| Benchmark suite | Previous | Current | Change |
|---|---|---|---|
| Push_3KB_snapshot/ (ns/op) | 22.65 ms | 21.93 ms | 🟢 -3.18% |
| Push_3KB_snapshot/ (B/op) | 891.64 KB | 897.27 KB | 🔴 +0.63% |
| Push_3KB_snapshot/ (allocs/op) | 8,772 allocs | 8,768 allocs | 🟢 -0.05% |
| Push_30KB_snapshot/ (ns/op) | 156.61 ms | 155.79 ms | 🟢 -0.53% |
| Push_30KB_snapshot/ (B/op) | 7.18 MB | 7.13 MB | 🟢 -0.77% |
| Push_30KB_snapshot/ (allocs/op) | 83,212 allocs | 83,159 allocs | 🟢 -0.06% |
| Pull_3KB_snapshot/ (ns/op) | 8.19 ms | 7.97 ms | 🟢 -2.69% |
| Pull_3KB_snapshot/ (B/op) | 1.01 MB | 1.01 MB | 🔴 +0.05% |
| Pull_3KB_snapshot/ (allocs/op) | 17,234 allocs | 17,235 allocs | ⚪ 0% |
| Pull_30KB_snapshot/ (ns/op) | 19.53 ms | 20.30 ms | 🔴 +3.92% |
| Pull_30KB_snapshot/ (B/op) | 8.36 MB | 8.37 MB | 🔴 +0.07% |
| Pull_30KB_snapshot/ (allocs/op) | 168,509 allocs | 168,508 allocs | ⚪ 0% |
BenchmarkSplayTree
| Benchmark suite | Previous | Current | Change |
|---|---|---|---|
| stress_test_100000/ (ns/op) | 0.17 ns | 0.17 ns | 🔴 +0.88% |
| 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.35 ns | 0.37 ns | 🔴 +5.47% |
| 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.54 ns | 0.53 ns | 🟢 -1.43% |
| 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 | 🔴 +0.55% |
| 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.49% |
| 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 | 🔴 +1.58% |
| 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 | 🔴 +9.91% |
| 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) | 7364.00 ns | 7400.00 ns | 🔴 +0.49% |
| memory_sync_10_test/ (B/op) | 1.34 KB | 1.34 KB | 🟢 -0.07% |
| memory_sync_10_test/ (allocs/op) | 35 allocs | 35 allocs | ⚪ 0% |
| memory_sync_100_test/ (ns/op) | 55300.00 ns | 56418.00 ns | 🔴 +2.02% |
| memory_sync_100_test/ (B/op) | 9.53 KB | 9.48 KB | 🟢 -0.46% |
| memory_sync_100_test/ (allocs/op) | 268 allocs | 267 allocs | 🟢 -0.37% |
| memory_sync_1000_test/ (ns/op) | 610683.00 ns | 622051.00 ns | 🔴 +1.86% |
| memory_sync_1000_test/ (B/op) | 76.07 KB | 75.66 KB | 🟢 -0.55% |
| memory_sync_1000_test/ (allocs/op) | 2,117 allocs | 2,104 allocs | 🟢 -0.61% |
| memory_sync_10000_test/ (ns/op) | 7.64 ms | 7.58 ms | 🟢 -0.84% |
| memory_sync_10000_test/ (B/op) | 750.26 KB | 762.99 KB | 🔴 +1.70% |
| memory_sync_10000_test/ (allocs/op) | 20,396 allocs | 20,546 allocs | 🔴 +0.74% |
BenchmarkSyncConcurrency
| Benchmark suite | Previous | Current | Change |
|---|---|---|---|
| 1-100-10/ (ns/op) | 15.45 s | 14.20 s | 🟢 -8.08% |
| 1-100-10/ (B/op) | 8.41 GB | 7.62 GB | 🟢 -9.31% |
| 1-100-10/ (allocs/op) | 176,411,372 allocs | 159,925,404 allocs | 🟢 -9.35% |
| 100-100-10/ (ns/op) | 28.17 s | 15.54 s | 🟢 -44.81% |
| 100-100-10/ (B/op) | 16.79 GB | 7.86 GB | 🟢 -53.18% |
| 100-100-10/ (allocs/op) | 338,236,227 allocs | 165,690,068 allocs | 🟢 -51.01% |
| 300_100-10/ (ns/op) | 54.91 s | 18.36 s | 🟢 -66.56% |
| 300_100-10/ (B/op) | 33.25 GB | 8.42 GB | 🟢 -74.67% |
| 300_100-10/ (allocs/op) | 660,524,835 allocs | 172,927,496 allocs | 🟢 -73.82% |
BenchmarkTextEditing
| Benchmark suite | Previous | Current | Change |
|---|---|---|---|
| (ns/op) | 5.83 s | 5.80 s | 🟢 -0.47% |
| (B/op) | 3.95 GB | 3.88 GB | 🟢 -1.54% |
| (allocs/op) | 21,372,006 allocs | 20,542,632 allocs | 🟢 -3.88% |
BenchmarkTree
| Benchmark suite | Previous | Current | Change |
|---|---|---|---|
| 10000_vertices_to_protobuf/ (ns/op) | 4.76 ms | 4.80 ms | 🔴 +0.69% |
| 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) | 207.59 ms | 213.14 ms | 🔴 +2.67% |
| 10000_vertices_from_protobuf/ (B/op) | 442.15 MB | 442.15 MB | ⚪ 0% |
| 10000_vertices_from_protobuf/ (allocs/op) | 280,046 allocs | 280,064 allocs | ⚪ 0% |
| 20000_vertices_to_protobuf/ (ns/op) | 10.27 ms | 10.38 ms | 🔴 +1.08% |
| 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) | 848.27 ms | 848.51 ms | 🔴 +0.03% |
| 20000_vertices_from_protobuf/ (B/op) | 1.70 GB | 1.70 GB | ⚪ 0% |
| 20000_vertices_from_protobuf/ (allocs/op) | 560,077 allocs | 560,075 allocs | ⚪ 0% |
| 30000_vertices_to_protobuf/ (ns/op) | 15.59 ms | 16.41 ms | 🔴 +5.26% |
| 30000_vertices_to_protobuf/ (B/op) | 19.94 MB | 19.94 MB | ⚪ 0% |
| 30000_vertices_to_protobuf/ (allocs/op) | 270,030 allocs | 270,031 allocs | ⚪ 0% |
| 30000_vertices_from_protobuf/ (ns/op) | 1.93 s | 1.95 s | 🔴 +0.90% |
| 30000_vertices_from_protobuf/ (B/op) | 3.75 GB | 3.75 GB | ⚪ 0% |
| 30000_vertices_from_protobuf/ (allocs/op) | 840,222 allocs | 840,120 allocs | 🟢 -0.01% |
BenchmarkVersionVector
| Benchmark suite | Previous | Current | Change |
|---|---|---|---|
| clients_10/ (ns/op) | 132.90 ms | 131.66 ms | 🟢 -0.93% |
| clients_10/ (1_changepack(bytes)) | 745.00 B | 202.00 B | 🟢 -72.89% |
| clients_10/ (2_snapshot(bytes)) | 399.00 B | 399.00 B | ⚪ 0% |
| clients_10/ (3_pushpull(ms)) | 5.00 ms | 5.00 ms | ⚪ 0% |
| clients_10/ (4_attach(ms)) | 5.00 ms | 5.00 ms | ⚪ 0% |
| clients_10/ (5_changepack_after_detach(bytes)) | 805.00 B | 262.00 B | 🟢 -67.45% |
| clients_10/ (6_snapshot_after_detach(bytes)) | 156.00 B | 156.00 B | ⚪ 0% |
| clients_10/ (7_pushpull_after_detach(ms)) | 5.00 ms | 5.00 ms | ⚪ 0% |
| clients_10/ (B/op) | 18.11 MB | 17.90 MB | 🟢 -1.15% |
| clients_10/ (allocs/op) | 64,341 allocs | 61,120 allocs | 🟢 -5.01% |
| clients_100/ (ns/op) | 1.15 s | 1.15 s | 🔴 +0.24% |
| clients_100/ (1_changepack(bytes)) | 6.14 KB | 202.00 B | 🟢 -96.71% |
| clients_100/ (2_snapshot(bytes)) | 3.10 KB | 3.10 KB | ⚪ 0% |
| clients_100/ (3_pushpull(ms)) | 6.00 ms | 6.00 ms | ⚪ 0% |
| clients_100/ (4_attach(ms)) | 8.00 ms | 7.00 ms | 🟢 -12.50% |
| clients_100/ (5_changepack_after_detach(bytes)) | 6.21 KB | 267.00 B | 🟢 -95.70% |
| clients_100/ (6_snapshot_after_detach(bytes)) | 157.00 B | 157.00 B | ⚪ 0% |
| clients_100/ (7_pushpull_after_detach(ms)) | 6.00 ms | 6.00 ms | ⚪ 0% |
| clients_100/ (B/op) | 175.45 MB | 178.88 MB | 🔴 +1.95% |
| clients_100/ (allocs/op) | 879,629 allocs | 833,614 allocs | 🟢 -5.23% |
| clients_1000/ (ns/op) | 20.37 s | 15.88 s | 🟢 -22.07% |
| clients_1000/ (1_changepack(bytes)) | 60.16 KB | 208.00 B | 🟢 -99.65% |
| clients_1000/ (2_snapshot(bytes)) | 30.10 KB | 30.10 KB | ⚪ 0% |
| clients_1000/ (3_pushpull(ms)) | 17.00 ms | 9.00 ms | 🟢 -47.06% |
| clients_1000/ (4_attach(ms)) | 62.00 ms | 24.00 ms | 🟢 -61.29% |
| clients_1000/ (5_changepack_after_detach(bytes)) | 60.22 KB | 270.00 B | 🟢 -99.55% |
| clients_1000/ (6_snapshot_after_detach(bytes)) | 161.00 B | 161.00 B | ⚪ 0% |
| clients_1000/ (7_pushpull_after_detach(ms)) | 17.00 ms | 6.00 ms | 🟢 -64.71% |
| clients_1000/ (B/op) | 3.64 GB | 3.41 GB | 🟢 -6.49% |
| clients_1000/ (allocs/op) | 43,195,000 allocs | 41,728,081 allocs | 🟢 -3.40% |
BenchmarkWebhook
| Benchmark suite | Previous | Current | Change |
|---|---|---|---|
| Send_10_Webhooks_to_10_Endpoints/ (ns/op) | 12.35 ms | 12.35 ms | 🟢 -0.05% |
| Send_10_Webhooks_to_10_Endpoints/ (B/op) | 808.28 KB | 808.37 KB | 🔴 +0.01% |
| Send_10_Webhooks_to_10_Endpoints/ (allocs/op) | 10,116 allocs | 10,117 allocs | ⚪ 0% |
| Send_100_Webhooks_to_10_Endpoints/ (ns/op) | 122.95 ms | 123.56 ms | 🔴 +0.50% |
| Send_100_Webhooks_to_10_Endpoints/ (B/op) | 8.08 MB | 8.08 MB | 🔴 +0.03% |
| Send_100_Webhooks_to_10_Endpoints/ (allocs/op) | 101,157 allocs | 101,158 allocs | ⚪ 0% |
| Send_10_Webhooks_to_100_Endpoints/ (ns/op) | 124.52 ms | 125.93 ms | 🔴 +1.14% |
| Send_10_Webhooks_to_100_Endpoints/ (B/op) | 8.25 MB | 8.28 MB | 🔴 +0.31% |
| Send_10_Webhooks_to_100_Endpoints/ (allocs/op) | 102,368 allocs | 102,537 allocs | 🔴 +0.17% |
| Send_100_Webhooks_to_100_Endpoints/ (ns/op) | 1.24 s | 1.25 s | 🔴 +0.81% |
| Send_100_Webhooks_to_100_Endpoints/ (B/op) | 82.37 MB | 82.36 MB | ⚪ 0% |
| Send_100_Webhooks_to_100_Endpoints/ (allocs/op) | 1,022,471 allocs | 1,022,432 allocs | ⚪ 0% |
| Send_10_Webhooks_to_1000_Endpoints/ (ns/op) | 2.76 s | 2.79 s | 🔴 +1.25% |
| Send_10_Webhooks_to_1000_Endpoints/ (B/op) | 209.68 MB | 209.72 MB | 🔴 +0.02% |
| Send_10_Webhooks_to_1000_Endpoints/ (allocs/op) | 1,716,498 allocs | 1,716,616 allocs | ⚪ 0% |
BenchmarkWebhookWithLimit
| Benchmark suite | Previous | Current | Change |
|---|---|---|---|
| Send_10_Webhooks_to_10_Endpoints_with_limit/ (ns/op) | 3.72 ms | 3.75 ms | 🔴 +0.94% |
| Send_10_Webhooks_to_10_Endpoints_with_limit/ (B/op) | 306.26 KB | 306.38 KB | 🔴 +0.04% |
| Send_10_Webhooks_to_10_Endpoints_with_limit/ (allocs/op) | 2,912 allocs | 2,913 allocs | 🔴 +0.03% |
| Send_100_Webhooks_to_10_Endpoints_with_limit/ (ns/op) | 4.02 ms | 4.05 ms | 🔴 +0.80% |
| Send_100_Webhooks_to_10_Endpoints_with_limit/ (B/op) | 428.88 KB | 428.91 KB | ⚪ 0% |
| Send_100_Webhooks_to_10_Endpoints_with_limit/ (allocs/op) | 4,713 allocs | 4,714 allocs | 🔴 +0.02% |
| Send_10_Webhooks_to_100_Endpoints_with_limit/ (ns/op) | 37.17 ms | 38.26 ms | 🔴 +2.91% |
| Send_10_Webhooks_to_100_Endpoints_with_limit/ (B/op) | 3.08 MB | 3.08 MB | 🟢 -0.01% |
| Send_10_Webhooks_to_100_Endpoints_with_limit/ (allocs/op) | 29,169 allocs | 29,169 allocs | ⚪ 0% |
| Send_100_Webhooks_to_100_Endpoints_with_limit/ (ns/op) | 40.30 ms | 40.92 ms | 🔴 +1.54% |
| 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,184 allocs | 47,191 allocs | 🔴 +0.01% |
| Send_10_Webhooks_to_1000_Endpoints_with_limit/ (ns/op) | 364.87 ms | 370.83 ms | 🔴 +1.63% |
| Send_10_Webhooks_to_1000_Endpoints_with_limit/ (B/op) | 44.49 MB | 44.52 MB | 🔴 +0.06% |
| Send_10_Webhooks_to_1000_Endpoints_with_limit/ (allocs/op) | 380,172 allocs | 380,144 allocs | ⚪ 0% |
hackerwins
force-pushed
the
cleanup-lamport
branch
from
3406cf0 to
360b280
Compare
hackerwins
changed the title
What this PR does / why we need it:
This PR cleans up legacy fields and functionality that were retained for backward compatibility during the transition from Lamport timestamps to Version Vectors.
MinSyncedTicketfield and its associated logicMaxCreatedAtMapByActorfield and its associated logic🔍 Go Benchmark Performance Analysis
Overview
This report presents a performance analysis of recent optimizations following the removal of two legacy fields:
minSyncedTicketandmaxCreatedAtMap, originally kept for Lamport clock compatibility.From the benchmark summary, we observed ≥20% improvements across a wide range of test suites. The standout changes were in:
BenchmarkRPC/client_to_client_via_serverBenchmarkDocumentDeletion/concurrent_*scenarios (tree and text)These improvements can be attributed primarily to the removal of costly operations tied to
minSyncedTicket.1.
minSyncedTicketRemovalBefore:
pushPull) calledUpdateAndFindMinSyncedVersionVectorandUpdateAndFindMinSyncedTicket.After:
minSyncedTicket, this call is no longer required.BenchmarkRPC) show a ~30% performance improvement, validating this change.UpdateAndFindMinSyncedTicketnode disappeared entirely post-optimization.📌 Conclusion: This field had high runtime impact on sync paths and was a major bottleneck.
2.
maxCreatedAtMapRemovalContext:
Observations:
concurrent_tree_delete_range_*andconcurrent_text_delete_range_*show modest improvements (~22–24%).maxCreatedAtMaplogic as a major memory consumer.Flamegraph Details:
traverseInPosRangeand related CRDT logic.maxCreatedAtMapwas present, its runtime cost was relatively small or tightly integrated into other paths.📌 Conclusion: Removing
maxCreatedAtMapsimplifies the logic and reduces complexity, but performance gains are secondary compared tominSyncedTicket.📌 Summary
Removing the
minSyncedTicketandmaxCreatedAtMapfields:minSyncedTicket)maxCreatedAtMap)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
Refactor
Chores