Replace literals-based initialization with YSON#1253
Conversation
|
""" WalkthroughThe changes introduce a new Changes
Sequence Diagram(s)sequenceDiagram
participant Test as Test (yson_test.go)
participant YSON as yson.ParseArray
participant Array as yson.Array
Test->>YSON: ParseArray("[Text(), Tree()]")
YSON->>Array: Unmarshal input string to Array
Array-->>YSON: Return parsed Array
YSON-->>Test: Return Array with empty Text and Tree (default root type)
sequenceDiagram
participant Object as Object.SetNewArray
participant Array as Array
participant YSON as yson.Array
Object->>Array: Create new empty Array
alt argument provided
Object->>YSON: Iterate over yson.Array elements
YSON-->>Array: Add elements via AddYSON
end
Array-->>Object: Return initialized Array
Possibly related PRs
Suggested reviewers
Tip ⚡️ Faster reviews with caching
Enjoy the performance boost—your workflow just got faster. ✨ 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
🔭 Outside diff range comments (1)
test/integration/array_test.go (1)
31-458: 💡 Verification agent🧩 Analysis chain
Verify test coverage for removed tests
According to the AI summary, the PR removes the
TestArraySetTypeGuardandTestArraySettest functions. Since those were testing type guard behavior and array set operations with different input types, we should ensure that the functionality is still adequately tested with the new YSON approach.
🏁 Script executed:
#!/bin/bash # Check if there are any tests still verifying proper array type handling echo "Checking for tests verifying array type handling:" rg -A 5 "ParseArray|SetNewArray.*yson" --type go test/ # Check how many test cases we've lost vs. kept echo "Removed tests for array type handling:" git diff HEAD^ HEAD -- test/integration/array_test.go | grep -c "^-func Test" echo "Tests still covering array functionality:" grep -c "^func Test.*Array" test/integration/array_test.goLength of output: 2633
Ensure array type-guard behavior is still tested
We’ve removed two tests (
TestArraySetTypeGuardandTestArraySet) but still have three top-levelTest*Arrayfunctions covering nested updates, concurrency, index-based sets, YSON-based array construction, sync, and specialized types (counters, text, trees). However, there are no negative tests asserting that inserting a mismatched type into an array errors out.Please add back—or rework—tests in
test/integration/array_test.goto verify type-guard behavior, for example:
- Attempting to
AddStringon an integer array (or vice versa) and asserting an error is returned.- Using
SetNewArray/ParseArraywith mixed or invalid element types and verifying appropriate handling.Locations needing coverage (approximate):
- After existing
array.set with value add, delete test- Within a new subtest under
TestArrayor a dedicatedTestArrayTypeGuardfunction
🧹 Nitpick comments (2)
pkg/document/yson/yson.go (1)
555-563: Error handling in ParseArray could be improved.While the new helper function provides a convenient way to parse YSON arrays, using panic for error handling is generally not recommended in libraries. Consider returning the error instead to allow callers to handle errors gracefully.
-func ParseArray(data string) Array { +func ParseArray(data string) (Array, error) { var arr Array if err := Unmarshal(data, &arr); err != nil { - panic("parse array" + err.Error()) + return nil, fmt.Errorf("parse array: %w", err) } - return arr + return arr, nil }Also note the missing space in the error message between "parse array" and the error content.
pkg/document/yson/yson_test.go (1)
440-452: Good test coverage for the new ParseArray function.The test verifies that the new ParseArray function correctly handles empty constructors and assigns default values. It tests both the empty Text() and Tree() constructors, confirming they're properly converted to their respective YSON types with the DefaultRootNodeType assigned for Tree.
Consider adding more test cases to cover:
- Error handling behavior (invalid inputs)
- More complex array structures
- Edge cases with various combinations of YSON types
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
pkg/document/json/array.go(1 hunks)pkg/document/json/object.go(1 hunks)pkg/document/json/tree.go(1 hunks)pkg/document/yson/yson.go(3 hunks)pkg/document/yson/yson_test.go(1 hunks)test/integration/array_test.go(4 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (5)
pkg/document/yson/yson_test.go (1)
pkg/document/yson/yson.go (6)
ParseArray(557-563)Array(71-71)Text(109-111)Tree(95-97)TreeNode(77-92)DefaultRootNodeType(51-51)
pkg/document/json/tree.go (2)
pkg/document/crdt/tree.go (2)
NewTreeNode(95-107)NewTreeNodeID(87-92)pkg/document/yson/yson.go (1)
DefaultRootNodeType(51-51)
pkg/document/json/object.go (5)
pkg/document/yson/yson.go (3)
Object(74-74)Array(71-71)Element(57-62)pkg/document/json/array.go (2)
Array(33-36)NewArray(39-44)pkg/document/crdt/array.go (2)
Array(26-31)NewArray(34-44)pkg/document/time/ticket.go (1)
Ticket(58-68)pkg/document/crdt/rga_tree_list.go (1)
NewRGATreeList(109-126)
pkg/document/yson/yson.go (3)
pkg/document/json/tree.go (1)
TreeNode(44-44)pkg/document/crdt/tree.go (1)
TreeNode(135-151)pkg/document/json/array.go (1)
Array(33-36)
test/integration/array_test.go (1)
pkg/document/yson/yson.go (1)
ParseArray(557-563)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: build
- GitHub Check: complex-test
- GitHub Check: bench
🔇 Additional comments (16)
pkg/document/yson/yson.go (4)
49-52: Good addition of a reusable constant.The new
DefaultRootNodeTypeconstant centralizes the default root node type value, avoiding string literal duplication across the codebase. This supports the overall goal of replacing literals with YSON.
485-491: Improved robustness in node parsing.The modification to
parseTreeNodenow gracefully handles missing or invalid "type" fields by assigning the default root node type instead of potentially panicking. This makes the function more robust against malformed input data.
521-545: Good refactoring for ordered replacements.Refactoring the
preprocessTypeValuesfunction to use a slice of structs instead of a map ensures deterministic ordering of replacements, which is crucial for correctly handling nested types. The grouping and comments explaining the processing order improve code readability and maintainability.
548-550: Simplified replacement logic.The loop iteration is now cleaner and more straightforward by simply iterating through the defined replacements in order.
pkg/document/json/array.go (1)
251-251: Minor style improvement.Addition of a blank line before the return statement improves code readability by separating the logic from the return statement.
pkg/document/json/tree.go (1)
365-365: Consistent use of the centralized constant.Good change to use the centralized
yson.DefaultRootNodeTypeconstant instead of a local string literal or duplicate constant. This improves maintainability by ensuring the default root node type is defined in one place.pkg/document/json/object.go (4)
86-87: Implementation simplified to always create an empty array firstThe implementation now always initializes a new empty array with
RGATreeListregardless of whether variadic arguments are provided. This is more consistent than the previous approach that may have handled different types of inputs differently.
90-94: Elements added using AddYSON instead of direct assignmentThe method now adds elements from the provided YSON array using the
AddYSONmethod rather than directly assigning them. This ensures consistent handling of array elements through the YSON interface.
96-96: Always returns the created arrayThe method consistently returns the created array instance, which maintains the method's contract even with the simplified implementation.
84-84:Details
✅ Verification successful
API change: Parameter type changed from
...anyto...yson.ArrayThe method signature has been updated to accept only YSON arrays instead of arbitrary values. This is a breaking change that aligns with the PR objective of replacing literals with YSON.
🏁 Script executed:
#!/bin/bash # Check if there are any existing callers that pass non-YSON arrays rg "SetNewArray\(.+,.+" --type go | grep -v "yson.ParseArray" || echo "No direct non-YSON array calls found"Length of output: 1519
No breaking changes detected for SetNewArray signature
A search for direct callers passing non-YSON values intoSetNewArrayreturned only zero-arg usages (followed by.AddString/.AddInteger), so the new...yson.Arrayparameter doesn’t break any existing call sites.test/integration/array_test.go (6)
30-30: Import updated to include YSON packageAdded import for the
ysonpackage to support the new YSON parsing approach for arrays.
246-246: Array initialization updated to use YSON parsingChanged from directly initializing arrays with literals to using
yson.ParseArray("[Int(0),[Int(1),Int(2),Int(3)]]"). This aligns with the newSetNewArrayimplementation that only accepts YSON arrays.
277-277: Array initialization replaced with YSON parsingReplaced direct array creation with
yson.ParseArrayto align with the updatedSetNewArrayimplementation.
302-302: Counter array initialization updated to use YSONUpdated to create arrays of Counters using the YSON syntax through
ParseArrayinstead of directly passing Counter objects.
307-307: Text array initialization updated to use YSONUpdated to create arrays of Text objects using the YSON syntax through
ParseArrayinstead of directly passing Text objects.
312-312: Tree array initialization updated to use YSONUpdated to create arrays of Tree objects using the YSON syntax through
ParseArrayinstead of directly passing Tree objects.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1253 +/- ##
==========================================
- Coverage 37.47% 37.02% -0.45%
==========================================
Files 179 179
Lines 28179 27959 -220
==========================================
- Hits 10559 10353 -206
+ Misses 16745 16734 -11
+ Partials 875 872 -3 ☔ 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(c3652c5) and the current commit(9b37d12).
Significant Changes (≥20% difference)
Nothing
Key Observations 🔍
- Significant decrease in performance observed in the
BenchmarkPresenceConcurrencysuite:- The
300-100-10benchmark showed a 2.26% decrease in execution time. - Memory consumption increased slightly in benchmarks like
0-100-10and100-100-10.
- The
- Overall, there were no significant changes (≥20%) in performance across the benchmark suites.
- In general, most benchmarks showed minor fluctuations in performance, with some improvements and regressions, but no clear trends were identified.
Detailed Test Results
BenchmarkDeletionConcurrency
| Benchmark suite | Previous | Current | Change |
|---|---|---|---|
| concurrent_text_delete_range_100/ (ns/op) | 878.09 ms | 876.29 ms | 🟢 -0.21% |
| concurrent_text_delete_range_100/ (B/op) | 70.87 MB | 68.69 MB | 🟢 -3.08% |
| concurrent_text_delete_range_100/ (allocs/op) | 789,062 allocs | 788,911 allocs | 🟢 -0.02% |
| concurrent_text_delete_range_1000/ (ns/op) | 7.11 s | 7.17 s | 🔴 +0.78% |
| concurrent_text_delete_range_1000/ (B/op) | 400.32 MB | 391.67 MB | 🟢 -2.16% |
| concurrent_text_delete_range_1000/ (allocs/op) | 7,488,045 allocs | 7,486,920 allocs | 🟢 -0.02% |
| concurrent_tree_delete_range_100/ (ns/op) | 918.30 ms | 888.91 ms | 🟢 -3.20% |
| concurrent_tree_delete_range_100/ (B/op) | 78.39 MB | 78.49 MB | 🔴 +0.13% |
| concurrent_tree_delete_range_100/ (allocs/op) | 826,282 allocs | 826,227 allocs | ⚪ 0% |
| concurrent_tree_delete_range_1000/ (ns/op) | 7.76 s | 7.88 s | 🔴 +1.60% |
| concurrent_tree_delete_range_1000/ (B/op) | 910.82 MB | 913.18 MB | 🔴 +0.26% |
| concurrent_tree_delete_range_1000/ (allocs/op) | 7,792,383 allocs | 7,792,733 allocs | ⚪ 0% |
| concurrent_text_edit_delete_all_100/ (ns/op) | 753.40 ms | 756.10 ms | 🔴 +0.36% |
| concurrent_text_edit_delete_all_100/ (B/op) | 65.81 MB | 65.80 MB | ⚪ 0% |
| concurrent_text_edit_delete_all_100/ (allocs/op) | 658,327 allocs | 658,234 allocs | 🟢 -0.01% |
| concurrent_text_edit_delete_all_1000/ (ns/op) | 5.89 s | 5.96 s | 🔴 +1.15% |
| concurrent_text_edit_delete_all_1000/ (B/op) | 330.37 MB | 331.14 MB | 🔴 +0.23% |
| concurrent_text_edit_delete_all_1000/ (allocs/op) | 5,996,699 allocs | 5,996,830 allocs | ⚪ 0% |
| concurrent_tree_edit_delete_all_100/ (ns/op) | 765.88 ms | 771.07 ms | 🔴 +0.68% |
| concurrent_tree_edit_delete_all_100/ (B/op) | 68.51 MB | 68.64 MB | 🔴 +0.20% |
| concurrent_tree_edit_delete_all_100/ (allocs/op) | 698,949 allocs | 699,000 allocs | ⚪ 0% |
| concurrent_tree_edit_delete_all_1000/ (ns/op) | 6.44 s | 6.50 s | 🔴 +1.02% |
| concurrent_tree_edit_delete_all_1000/ (B/op) | 729.93 MB | 725.98 MB | 🟢 -0.54% |
| concurrent_tree_edit_delete_all_1000/ (allocs/op) | 6,402,241 allocs | 6,402,378 allocs | ⚪ 0% |
BenchmarkDocument
| Benchmark suite | Previous | Current | Change |
|---|---|---|---|
| constructor_test/ (ns/op) | 1499.00 ns | 1490.00 ns | 🟢 -0.60% |
| constructor_test/ (B/op) | 1.45 KB | 1.45 KB | ⚪ 0% |
| constructor_test/ (allocs/op) | 25 allocs | 25 allocs | ⚪ 0% |
| status_test/ (ns/op) | 1100.00 ns | 1089.00 ns | 🟢 -1.00% |
| status_test/ (B/op) | 1.42 KB | 1.42 KB | ⚪ 0% |
| status_test/ (allocs/op) | 23 allocs | 23 allocs | ⚪ 0% |
| equals_test/ (ns/op) | 8209.00 ns | 8139.00 ns | 🟢 -0.85% |
| 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) | 17541.00 ns | 17627.00 ns | 🔴 +0.49% |
| 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) | 23854.00 ns | 23778.00 ns | 🟢 -0.32% |
| delete_test/ (B/op) | 16.20 KB | 16.20 KB | ⚪ 0% |
| delete_test/ (allocs/op) | 350 allocs | 350 allocs | ⚪ 0% |
| object_test/ (ns/op) | 9047.00 ns | 9049.00 ns | 🔴 +0.02% |
| object_test/ (B/op) | 7.22 KB | 7.22 KB | ⚪ 0% |
| object_test/ (allocs/op) | 123 allocs | 123 allocs | ⚪ 0% |
| array_test/ (ns/op) | 29579.00 ns | 29691.00 ns | 🔴 +0.38% |
| array_test/ (B/op) | 12.51 KB | 12.51 KB | ⚪ 0% |
| array_test/ (allocs/op) | 282 allocs | 282 allocs | ⚪ 0% |
| text_test/ (ns/op) | 33682.00 ns | 33614.00 ns | 🟢 -0.20% |
| text_test/ (B/op) | 15.97 KB | 15.97 KB | ⚪ 0% |
| text_test/ (allocs/op) | 513 allocs | 513 allocs | ⚪ 0% |
| text_composition_test/ (ns/op) | 33328.00 ns | 33319.00 ns | 🟢 -0.03% |
| text_composition_test/ (B/op) | 19.24 KB | 19.24 KB | ⚪ 0% |
| text_composition_test/ (allocs/op) | 517 allocs | 517 allocs | ⚪ 0% |
| rich_text_test/ (ns/op) | 89203.00 ns | 90108.00 ns | 🔴 +1.01% |
| rich_text_test/ (B/op) | 40.96 KB | 40.96 KB | ⚪ 0% |
| rich_text_test/ (allocs/op) | 1,207 allocs | 1,207 allocs | ⚪ 0% |
| counter_test/ (ns/op) | 18871.00 ns | 18447.00 ns | 🟢 -2.25% |
| counter_test/ (B/op) | 12.37 KB | 12.18 KB | 🟢 -1.55% |
| counter_test/ (allocs/op) | 264 allocs | 260 allocs | 🟢 -1.52% |
| text_edit_gc_100/ (ns/op) | 1.49 ms | 1.49 ms | 🔴 +0.01% |
| text_edit_gc_100/ (B/op) | 875.53 KB | 875.50 KB | ⚪ 0% |
| text_edit_gc_100/ (allocs/op) | 17,588 allocs | 17,588 allocs | ⚪ 0% |
| text_edit_gc_1000/ (ns/op) | 56.96 ms | 57.77 ms | 🔴 +1.43% |
| text_edit_gc_1000/ (B/op) | 46.94 MB | 46.94 MB | ⚪ 0% |
| text_edit_gc_1000/ (allocs/op) | 188,602 allocs | 188,593 allocs | ⚪ 0% |
| text_split_gc_100/ (ns/op) | 2.21 ms | 2.22 ms | 🔴 +0.52% |
| text_split_gc_100/ (B/op) | 1.58 MB | 1.58 MB | ⚪ 0% |
| text_split_gc_100/ (allocs/op) | 15,958 allocs | 15,960 allocs | 🔴 +0.01% |
| text_split_gc_1000/ (ns/op) | 137.30 ms | 135.21 ms | 🟢 -1.52% |
| text_split_gc_1000/ (B/op) | 137.81 MB | 137.80 MB | ⚪ 0% |
| text_split_gc_1000/ (allocs/op) | 184,996 allocs | 185,003 allocs | ⚪ 0% |
| text_100/ (ns/op) | 241002.00 ns | 243814.00 ns | 🔴 +1.17% |
| text_100/ (B/op) | 122.66 KB | 122.66 KB | ⚪ 0% |
| text_100/ (allocs/op) | 5,185 allocs | 5,185 allocs | ⚪ 0% |
| text_1000/ (ns/op) | 2.62 ms | 2.60 ms | 🟢 -0.65% |
| text_1000/ (B/op) | 1.17 MB | 1.17 MB | ⚪ 0% |
| text_1000/ (allocs/op) | 51,088 allocs | 51,088 allocs | ⚪ 0% |
| array_1000/ (ns/op) | 1.31 ms | 1.30 ms | 🟢 -0.73% |
| array_1000/ (B/op) | 1.13 MB | 1.13 MB | ⚪ 0% |
| array_1000/ (allocs/op) | 12,884 allocs | 12,883 allocs | ⚪ 0% |
| array_10000/ (ns/op) | 14.08 ms | 14.45 ms | 🔴 +2.58% |
| array_10000/ (B/op) | 10.29 MB | 10.29 MB | ⚪ 0% |
| array_10000/ (allocs/op) | 130,736 allocs | 130,732 allocs | ⚪ 0% |
| array_gc_100/ (ns/op) | 138896.00 ns | 140068.00 ns | 🔴 +0.84% |
| array_gc_100/ (B/op) | 104.10 KB | 104.10 KB | ⚪ 0% |
| array_gc_100/ (allocs/op) | 1,372 allocs | 1,372 allocs | ⚪ 0% |
| array_gc_1000/ (ns/op) | 1.47 ms | 1.48 ms | 🔴 +0.39% |
| 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) | 206736.00 ns | 206580.00 ns | 🟢 -0.08% |
| counter_1000/ (B/op) | 194.29 KB | 194.19 KB | 🟢 -0.05% |
| counter_1000/ (allocs/op) | 5,775 allocs | 5,773 allocs | 🟢 -0.03% |
| counter_10000/ (ns/op) | 2.25 ms | 2.22 ms | 🟢 -1.26% |
| counter_10000/ (B/op) | 2.23 MB | 2.23 MB | ⚪ 0% |
| counter_10000/ (allocs/op) | 59,782 allocs | 59,780 allocs | ⚪ 0% |
| object_1000/ (ns/op) | 1.52 ms | 1.50 ms | 🟢 -1.10% |
| object_1000/ (B/op) | 1.48 MB | 1.48 MB | ⚪ 0% |
| object_1000/ (allocs/op) | 10,928 allocs | 10,928 allocs | ⚪ 0% |
| object_10000/ (ns/op) | 16.66 ms | 16.49 ms | 🟢 -1.03% |
| object_10000/ (B/op) | 12.75 MB | 12.75 MB | ⚪ 0% |
| object_10000/ (allocs/op) | 111,238 allocs | 111,236 allocs | ⚪ 0% |
| tree_100/ (ns/op) | 790300.00 ns | 759197.00 ns | 🟢 -3.94% |
| tree_100/ (B/op) | 534.70 KB | 534.71 KB | ⚪ 0% |
| tree_100/ (allocs/op) | 4,716 allocs | 4,716 allocs | ⚪ 0% |
| tree_1000/ (ns/op) | 52.99 ms | 50.97 ms | 🟢 -3.81% |
| tree_1000/ (B/op) | 43.85 MB | 43.85 MB | ⚪ 0% |
| tree_1000/ (allocs/op) | 46,125 allocs | 46,125 allocs | ⚪ 0% |
| tree_10000/ (ns/op) | 6.72 s | 6.74 s | 🔴 +0.32% |
| tree_10000/ (B/op) | 4.30 GB | 4.30 GB | ⚪ 0% |
| tree_10000/ (allocs/op) | 460,184 allocs | 460,197 allocs | ⚪ 0% |
| tree_edit_gc_100/ (ns/op) | 2.82 ms | 2.77 ms | 🟢 -1.84% |
| tree_edit_gc_100/ (B/op) | 2.47 MB | 2.47 MB | ⚪ 0% |
| tree_edit_gc_100/ (allocs/op) | 11,565 allocs | 11,565 allocs | ⚪ 0% |
| tree_edit_gc_1000/ (ns/op) | 220.65 ms | 207.60 ms | 🟢 -5.92% |
| tree_edit_gc_1000/ (B/op) | 214.66 MB | 214.66 MB | ⚪ 0% |
| tree_edit_gc_1000/ (allocs/op) | 115,148 allocs | 115,142 allocs | ⚪ 0% |
| tree_split_gc_100/ (ns/op) | 2.02 ms | 1.97 ms | 🟢 -2.38% |
| tree_split_gc_100/ (B/op) | 1.55 MB | 1.55 MB | ⚪ 0% |
| tree_split_gc_100/ (allocs/op) | 8,538 allocs | 8,538 allocs | ⚪ 0% |
| tree_split_gc_1000/ (ns/op) | 143.79 ms | 136.34 ms | 🟢 -5.18% |
| tree_split_gc_1000/ (B/op) | 137.62 MB | 137.62 MB | ⚪ 0% |
| tree_split_gc_1000/ (allocs/op) | 95,891 allocs | 95,894 allocs | ⚪ 0% |
BenchmarkDocumentDeletion
| Benchmark suite | Previous | Current | Change |
|---|---|---|---|
| single_text_delete_all_10000/ (ns/op) | 18.40 ms | 18.82 ms | 🔴 +2.29% |
| single_text_delete_all_10000/ (B/op) | 11.29 MB | 11.30 MB | 🔴 +0.01% |
| single_text_delete_all_10000/ (allocs/op) | 86,131 allocs | 86,136 allocs | ⚪ 0% |
| single_text_delete_all_100000/ (ns/op) | 256.99 ms | 277.75 ms | 🔴 +8.08% |
| single_text_delete_all_100000/ (B/op) | 112.71 MB | 112.73 MB | 🔴 +0.02% |
| single_text_delete_all_100000/ (allocs/op) | 866,041 allocs | 866,091 allocs | ⚪ 0% |
| single_tree_delete_all_1000/ (ns/op) | 55.95 ms | 52.12 ms | 🟢 -6.84% |
| single_tree_delete_all_1000/ (B/op) | 44.84 MB | 44.84 MB | ⚪ 0% |
| single_tree_delete_all_1000/ (allocs/op) | 55,203 allocs | 55,203 allocs | ⚪ 0% |
| single_text_delete_range_100/ (ns/op) | 477647.00 ns | 457742.00 ns | 🟢 -4.17% |
| single_text_delete_range_100/ (B/op) | 264.23 KB | 264.23 KB | ⚪ 0% |
| single_text_delete_range_100/ (allocs/op) | 7,417 allocs | 7,417 allocs | ⚪ 0% |
| single_text_delete_range_1000/ (ns/op) | 9.25 ms | 9.16 ms | 🟢 -0.94% |
| single_text_delete_range_1000/ (B/op) | 6.59 MB | 6.59 MB | ⚪ 0% |
| single_text_delete_range_1000/ (allocs/op) | 76,772 allocs | 76,773 allocs | ⚪ 0% |
| single_tree_delete_range_100/ (ns/op) | 1.01 ms | 975098.00 ns | 🟢 -3.84% |
| single_tree_delete_range_100/ (B/op) | 731.44 KB | 731.49 KB | ⚪ 0% |
| single_tree_delete_range_100/ (allocs/op) | 6,129 allocs | 6,129 allocs | ⚪ 0% |
| single_tree_delete_range_1000/ (ns/op) | 65.76 ms | 60.70 ms | 🟢 -7.69% |
| single_tree_delete_range_1000/ (B/op) | 54.74 MB | 54.74 MB | ⚪ 0% |
| single_tree_delete_range_1000/ (allocs/op) | 60,832 allocs | 60,832 allocs | ⚪ 0% |
BenchmarkRPC
| Benchmark suite | Previous | Current | Change |
|---|---|---|---|
| client_to_server/ (ns/op) | 295.56 ms | 294.78 ms | 🟢 -0.26% |
| client_to_server/ (B/op) | 12.97 MB | 13.33 MB | 🔴 +2.80% |
| client_to_server/ (allocs/op) | 171,745 allocs | 171,104 allocs | 🟢 -0.37% |
| client_to_client_via_server/ (ns/op) | 356.24 ms | 355.56 ms | 🟢 -0.19% |
| client_to_client_via_server/ (B/op) | 19.44 MB | 20.20 MB | 🔴 +3.89% |
| client_to_client_via_server/ (allocs/op) | 280,784 allocs | 281,221 allocs | 🔴 +0.16% |
| attach_large_document/ (ns/op) | 1.36 s | 1.37 s | 🔴 +0.30% |
| attach_large_document/ (B/op) | 1.88 GB | 1.86 GB | 🟢 -1.26% |
| attach_large_document/ (allocs/op) | 11,158 allocs | 11,129 allocs | 🟢 -0.26% |
| adminCli_to_server/ (ns/op) | 564.94 ms | 570.30 ms | 🔴 +0.95% |
| adminCli_to_server/ (B/op) | 21.49 MB | 22.74 MB | 🔴 +5.82% |
| adminCli_to_server/ (allocs/op) | 316,711 allocs | 316,709 allocs | ⚪ 0% |
BenchmarkLocker
| Benchmark suite | Previous | Current | Change |
|---|---|---|---|
| (ns/op) | 83.62 ns | 81.34 ns | 🟢 -2.73% |
| (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.84 ns | 46.24 ns | 🔴 +0.87% |
| (B/op) | 0.00 B | 0.00 B | ⚪ 0% |
| (allocs/op) | 0 allocs | 0 allocs | ⚪ 0% |
BenchmarkLockerMoreKeys
| Benchmark suite | Previous | Current | Change |
|---|---|---|---|
| (ns/op) | 169.10 ns | 177.60 ns | 🔴 +5.03% |
| (B/op) | 31.00 B | 31.00 B | ⚪ 0% |
| (allocs/op) | 0 allocs | 0 allocs | ⚪ 0% |
BenchmarkRWLocker
| Benchmark suite | Previous | Current | Change |
|---|---|---|---|
| RWLock_rate_2/ (ns/op) | 50.41 ns | 50.29 ns | 🟢 -0.24% |
| 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) | 45.54 ns | 44.75 ns | 🟢 -1.73% |
| 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.75 ns | 61.63 ns | 🟢 -0.19% |
| 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) | 89.57 ns | 89.54 ns | 🟢 -0.03% |
| 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.77 s | 🟢 -2.26% |
| 0-100-10/ (B/op) | 426.44 MB | 428.35 MB | 🔴 +0.45% |
| 0-100-10/ (allocs/op) | 5,601,428 allocs | 5,605,773 allocs | 🔴 +0.08% |
| 100-100-10/ (ns/op) | 3.20 s | 3.16 s | 🟢 -1.25% |
| 100-100-10/ (B/op) | 708.33 MB | 704.54 MB | 🟢 -0.53% |
| 100-100-10/ (allocs/op) | 8,280,670 allocs | 8,277,899 allocs | 🟢 -0.03% |
| 300-100-10/ (ns/op) | 6.56 s | 6.57 s | 🔴 +0.22% |
| 300-100-10/ (B/op) | 1.34 GB | 1.35 GB | 🔴 +0.54% |
| 300-100-10/ (allocs/op) | 15,911,093 allocs | 15,907,716 allocs | 🟢 -0.02% |
BenchmarkChange
| Benchmark suite | Previous | Current | Change |
|---|---|---|---|
| Push_10_Changes/ (ns/op) | 4.39 ms | 4.38 ms | 🟢 -0.16% |
| Push_10_Changes/ (B/op) | 140.36 KB | 140.58 KB | 🔴 +0.16% |
| Push_10_Changes/ (allocs/op) | 1,523 allocs | 1,526 allocs | 🔴 +0.20% |
| Push_100_Changes/ (ns/op) | 18.37 ms | 18.26 ms | 🟢 -0.60% |
| Push_100_Changes/ (B/op) | 762.52 KB | 766.86 KB | 🔴 +0.57% |
| Push_100_Changes/ (allocs/op) | 8,766 allocs | 8,766 allocs | ⚪ 0% |
| Push_1000_Changes/ (ns/op) | 149.82 ms | 150.44 ms | 🔴 +0.41% |
| Push_1000_Changes/ (B/op) | 7.08 MB | 7.25 MB | 🔴 +2.47% |
| Push_1000_Changes/ (allocs/op) | 83,145 allocs | 83,147 allocs | ⚪ 0% |
| Pull_10_Changes/ (ns/op) | 3.21 ms | 3.21 ms | 🔴 +0.06% |
| Pull_10_Changes/ (B/op) | 105.93 KB | 105.96 KB | 🔴 +0.03% |
| Pull_10_Changes/ (allocs/op) | 1,120 allocs | 1,121 allocs | 🔴 +0.09% |
| Pull_100_Changes/ (ns/op) | 5.36 ms | 5.35 ms | 🟢 -0.29% |
| Pull_100_Changes/ (B/op) | 339.59 KB | 339.29 KB | 🟢 -0.09% |
| Pull_100_Changes/ (allocs/op) | 4,759 allocs | 4,758 allocs | 🟢 -0.02% |
| Pull_1000_Changes/ (ns/op) | 10.99 ms | 11.22 ms | 🔴 +2.08% |
| Pull_1000_Changes/ (B/op) | 2.16 MB | 2.15 MB | 🟢 -0.19% |
| Pull_1000_Changes/ (allocs/op) | 43,371 allocs | 43,369 allocs | ⚪ 0% |
BenchmarkSnapshot
| Benchmark suite | Previous | Current | Change |
|---|---|---|---|
| Push_3KB_snapshot/ (ns/op) | 21.47 ms | 22.05 ms | 🔴 +2.70% |
| Push_3KB_snapshot/ (B/op) | 910.46 KB | 901.82 KB | 🟢 -0.95% |
| Push_3KB_snapshot/ (allocs/op) | 8,770 allocs | 8,772 allocs | 🔴 +0.02% |
| Push_30KB_snapshot/ (ns/op) | 153.52 ms | 155.05 ms | 🔴 +1.00% |
| Push_30KB_snapshot/ (B/op) | 7.18 MB | 7.26 MB | 🔴 +1.04% |
| Push_30KB_snapshot/ (allocs/op) | 83,151 allocs | 83,159 allocs | ⚪ 0% |
| Pull_3KB_snapshot/ (ns/op) | 7.72 ms | 7.81 ms | 🔴 +1.23% |
| Pull_3KB_snapshot/ (B/op) | 1.01 MB | 1.01 MB | 🟢 -0.03% |
| Pull_3KB_snapshot/ (allocs/op) | 17,230 allocs | 17,233 allocs | 🔴 +0.02% |
| Pull_30KB_snapshot/ (ns/op) | 19.32 ms | 19.42 ms | 🔴 +0.50% |
| Pull_30KB_snapshot/ (B/op) | 8.37 MB | 8.36 MB | 🟢 -0.05% |
| Pull_30KB_snapshot/ (allocs/op) | 168,505 allocs | 168,506 allocs | ⚪ 0% |
BenchmarkSplayTree
| Benchmark suite | Previous | Current | Change |
|---|---|---|---|
| stress_test_100000/ (ns/op) | 0.19 ns | 0.17 ns | 🟢 -9.37% |
| 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.38 ns | 0.37 ns | 🟢 -1.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.57 ns | 0.56 ns | 🟢 -2.17% |
| 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 | 🟢 -2.81% |
| 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 | 🟢 -4.47% |
| 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.04 ns | 🟢 -6.07% |
| 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 | 🟢 -8.93% |
| 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) | 7472.00 ns | 7218.00 ns | 🟢 -3.40% |
| 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) | 55131.00 ns | 55587.00 ns | 🔴 +0.83% |
| memory_sync_100_test/ (B/op) | 9.50 KB | 9.48 KB | 🟢 -0.27% |
| memory_sync_100_test/ (allocs/op) | 268 allocs | 267 allocs | 🟢 -0.37% |
| memory_sync_1000_test/ (ns/op) | 609213.00 ns | 598530.00 ns | 🟢 -1.75% |
| memory_sync_1000_test/ (B/op) | 75.53 KB | 76.32 KB | 🔴 +1.05% |
| memory_sync_1000_test/ (allocs/op) | 2,101 allocs | 2,124 allocs | 🔴 +1.09% |
| memory_sync_10000_test/ (ns/op) | 7.45 ms | 7.68 ms | 🔴 +3.14% |
| memory_sync_10000_test/ (B/op) | 753.51 KB | 759.99 KB | 🔴 +0.86% |
| memory_sync_10000_test/ (allocs/op) | 20,384 allocs | 20,518 allocs | 🔴 +0.66% |
BenchmarkSyncConcurrency
| Benchmark suite | Previous | Current | Change |
|---|---|---|---|
| 1-100-10/ (ns/op) | 15.29 s | 15.62 s | 🔴 +2.14% |
| 1-100-10/ (B/op) | 8.41 GB | 8.43 GB | 🔴 +0.19% |
| 1-100-10/ (allocs/op) | 176,485,618 allocs | 176,479,672 allocs | ⚪ 0% |
| 100-100-10/ (ns/op) | 28.06 s | 28.33 s | 🔴 +0.96% |
| 100-100-10/ (B/op) | 16.80 GB | 16.80 GB | ⚪ 0% |
| 100-100-10/ (allocs/op) | 338,298,618 allocs | 338,170,373 allocs | 🟢 -0.04% |
| 300_100-10/ (ns/op) | 54.39 s | 54.51 s | 🔴 +0.22% |
| 300_100-10/ (B/op) | 33.26 GB | 33.24 GB | 🟢 -0.06% |
| 300_100-10/ (allocs/op) | 660,549,636 allocs | 660,572,141 allocs | ⚪ 0% |
BenchmarkTextEditing
| Benchmark suite | Previous | Current | Change |
|---|---|---|---|
| (ns/op) | 5.83 s | 5.80 s | 🟢 -0.46% |
| (B/op) | 3.94 GB | 3.94 GB | ⚪ 0% |
| (allocs/op) | 21,371,915 allocs | 21,371,950 allocs | ⚪ 0% |
BenchmarkTree
| Benchmark suite | Previous | Current | Change |
|---|---|---|---|
| 10000_vertices_to_protobuf/ (ns/op) | 4.62 ms | 4.74 ms | 🔴 +2.66% |
| 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) | 204.92 ms | 208.15 ms | 🔴 +1.58% |
| 10000_vertices_from_protobuf/ (B/op) | 442.14 MB | 442.15 MB | ⚪ 0% |
| 10000_vertices_from_protobuf/ (allocs/op) | 280,046 allocs | 280,064 allocs | ⚪ 0% |
| 20000_vertices_to_protobuf/ (ns/op) | 9.98 ms | 9.84 ms | 🟢 -1.38% |
| 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) | 834.00 ms | 849.78 ms | 🔴 +1.89% |
| 20000_vertices_from_protobuf/ (B/op) | 1.70 GB | 1.70 GB | ⚪ 0% |
| 20000_vertices_from_protobuf/ (allocs/op) | 560,121 allocs | 560,077 allocs | ⚪ 0% |
| 30000_vertices_to_protobuf/ (ns/op) | 15.00 ms | 15.34 ms | 🔴 +2.27% |
| 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.91 s | 1.96 s | 🔴 +2.60% |
| 30000_vertices_from_protobuf/ (B/op) | 3.75 GB | 3.75 GB | ⚪ 0% |
| 30000_vertices_from_protobuf/ (allocs/op) | 840,122 allocs | 840,127 allocs | ⚪ 0% |
BenchmarkVersionVector
| Benchmark suite | Previous | Current | Change |
|---|---|---|---|
| clients_10/ (ns/op) | 130.59 ms | 132.78 ms | 🔴 +1.67% |
| clients_10/ (1_changepack(bytes)) | 745.00 B | 745.00 B | ⚪ 0% |
| 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 | 805.00 B | ⚪ 0% |
| 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) | 17.72 MB | 17.22 MB | 🟢 -2.79% |
| clients_10/ (allocs/op) | 64,330 allocs | 64,318 allocs | 🟢 -0.02% |
| clients_100/ (ns/op) | 1.13 s | 1.15 s | 🔴 +1.27% |
| clients_100/ (1_changepack(bytes)) | 6.14 KB | 6.14 KB | ⚪ 0% |
| 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 | 8.00 ms | ⚪ 0% |
| clients_100/ (5_changepack_after_detach(bytes)) | 6.21 KB | 6.21 KB | ⚪ 0% |
| clients_100/ (6_snapshot_after_detach(bytes)) | 157.00 B | 157.00 B | ⚪ 0% |
| clients_100/ (7_pushpull_after_detach(ms)) | 6.00 ms | 7.00 ms | 🔴 +16.67% |
| clients_100/ (B/op) | 177.80 MB | 177.81 MB | ⚪ 0% |
| clients_100/ (allocs/op) | 879,549 allocs | 879,712 allocs | 🔴 +0.02% |
| clients_1000/ (ns/op) | 20.25 s | 20.56 s | 🔴 +1.55% |
| clients_1000/ (1_changepack(bytes)) | 60.16 KB | 60.16 KB | ⚪ 0% |
| clients_1000/ (2_snapshot(bytes)) | 30.10 KB | 30.10 KB | ⚪ 0% |
| clients_1000/ (3_pushpull(ms)) | 16.00 ms | 16.00 ms | ⚪ 0% |
| clients_1000/ (4_attach(ms)) | 64.00 ms | 68.00 ms | 🔴 +6.25% |
| clients_1000/ (5_changepack_after_detach(bytes)) | 60.22 KB | 60.22 KB | ⚪ 0% |
| clients_1000/ (6_snapshot_after_detach(bytes)) | 161.00 B | 161.00 B | ⚪ 0% |
| clients_1000/ (7_pushpull_after_detach(ms)) | 17.00 ms | 17.00 ms | ⚪ 0% |
| clients_1000/ (B/op) | 3.64 GB | 3.64 GB | 🔴 +0.06% |
| clients_1000/ (allocs/op) | 43,194,920 allocs | 43,194,042 allocs | ⚪ 0% |
BenchmarkWebhook
| Benchmark suite | Previous | Current | Change |
|---|---|---|---|
| Send_10_Webhooks_to_10_Endpoints/ (ns/op) | 12.12 ms | 12.15 ms | 🔴 +0.25% |
| Send_10_Webhooks_to_10_Endpoints/ (B/op) | 808.02 KB | 808.13 KB | 🔴 +0.01% |
| Send_10_Webhooks_to_10_Endpoints/ (allocs/op) | 10,114 allocs | 10,114 allocs | ⚪ 0% |
| Send_100_Webhooks_to_10_Endpoints/ (ns/op) | 121.93 ms | 121.33 ms | 🟢 -0.49% |
| 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,165 allocs | 101,153 allocs | 🟢 -0.01% |
| Send_10_Webhooks_to_100_Endpoints/ (ns/op) | 123.96 ms | 123.14 ms | 🟢 -0.66% |
| Send_10_Webhooks_to_100_Endpoints/ (B/op) | 8.26 MB | 8.26 MB | 🔴 +0.03% |
| Send_10_Webhooks_to_100_Endpoints/ (allocs/op) | 102,368 allocs | 102,369 allocs | ⚪ 0% |
| Send_100_Webhooks_to_100_Endpoints/ (ns/op) | 1.23 s | 1.23 s | 🟢 -0.31% |
| Send_100_Webhooks_to_100_Endpoints/ (B/op) | 82.37 MB | 82.37 MB | ⚪ 0% |
| Send_100_Webhooks_to_100_Endpoints/ (allocs/op) | 1,022,330 allocs | 1,022,408 allocs | ⚪ 0% |
| Send_10_Webhooks_to_1000_Endpoints/ (ns/op) | 2.98 s | 2.74 s | 🟢 -7.81% |
| Send_10_Webhooks_to_1000_Endpoints/ (B/op) | 209.31 MB | 209.30 MB | ⚪ 0% |
| Send_10_Webhooks_to_1000_Endpoints/ (allocs/op) | 1,716,407 allocs | 1,716,311 allocs | ⚪ 0% |
BenchmarkWebhookWithLimit
| Benchmark suite | Previous | Current | Change |
|---|---|---|---|
| Send_10_Webhooks_to_10_Endpoints_with_limit/ (ns/op) | 3.94 ms | 3.70 ms | 🟢 -6.16% |
| Send_10_Webhooks_to_10_Endpoints_with_limit/ (B/op) | 306.44 KB | 306.31 KB | 🟢 -0.04% |
| 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.23 ms | 3.98 ms | 🟢 -5.95% |
| Send_100_Webhooks_to_10_Endpoints_with_limit/ (B/op) | 428.87 KB | 428.78 KB | 🟢 -0.02% |
| 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) | 39.75 ms | 37.22 ms | 🟢 -6.36% |
| 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,180 allocs | 29,174 allocs | 🟢 -0.02% |
| Send_100_Webhooks_to_100_Endpoints_with_limit/ (ns/op) | 42.72 ms | 40.33 ms | 🟢 -5.58% |
| 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,190 allocs | 47,183 allocs | 🟢 -0.01% |
| Send_10_Webhooks_to_1000_Endpoints_with_limit/ (ns/op) | 398.43 ms | 363.90 ms | 🟢 -8.67% |
| Send_10_Webhooks_to_1000_Endpoints_with_limit/ (B/op) | 44.52 MB | 44.85 MB | 🔴 +0.75% |
| Send_10_Webhooks_to_1000_Endpoints_with_limit/ (allocs/op) | 380,178 allocs | 380,325 allocs | 🔴 +0.04% |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
pkg/document/yson/yson.go (3)
49-52: Add inline documentation for exported constant
DefaultRootNodeTypeis exported and therefore part of the public API. Please add a short comment that starts with the identifier’s name so thatgo vetandgolintdon’t complain and future readers quickly understand its purpose.-// DefaultRootNodeType is the default type of root node. -DefaultRootNodeType = "root" +// DefaultRootNodeType represents the fallback value that is assigned to a +// tree node when the incoming YSON does not specify an explicit `"type"` +// field. It is used only during deserialization. +DefaultRootNodeType = "root"
555-564: Panicking helper should be prefixed withMustor return an error
ParseArraypanics on invalid input but is named like an ordinary parser.
Either:
- Rename to
MustParseArray(idiomatic Go), or- Change the signature to
(Array, error)and let the caller decide.-func ParseArray(data string) Array { +func MustParseArray(data string) Array {Same remark applies to
ParseObjectbelow.
565-573: See comment above regarding panic-on-error APIPlease align
ParseObjectwith the decision taken forParseArrayto keep the public surface consistent.test/integration/object_test.go (1)
31-31: Import groupingNit:
ysonis a Yorkie package, so it could stay with the other Yorkie
imports to avoid an extra empty line."github.com/yorkie-team/yorkie/pkg/document/presence" -"github.com/yorkie-team/yorkie/pkg/document/yson" ... +"github.com/yorkie-team/yorkie/pkg/document/yson"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
pkg/document/crdt/primitive.go(2 hunks)pkg/document/json/object.go(1 hunks)pkg/document/yson/yson.go(3 hunks)test/integration/gc_test.go(2 hunks)test/integration/object_test.go(7 hunks)
✅ Files skipped from review due to trivial changes (1)
- test/integration/gc_test.go
🚧 Files skipped from review as they are similar to previous changes (1)
- pkg/document/json/object.go
🧰 Additional context used
🧬 Code Graph Analysis (2)
test/integration/object_test.go (4)
pkg/document/yson/yson.go (2)
ParseObject(567-573)Object(74-74)pkg/document/json/object.go (1)
Object(35-38)pkg/document/crdt/object.go (1)
Object(26-31)pkg/document/innerpresence/presence.go (1)
Presence(89-89)
pkg/document/yson/yson.go (4)
pkg/document/json/tree.go (1)
TreeNode(44-44)pkg/document/crdt/tree.go (1)
TreeNode(135-151)pkg/document/json/array.go (1)
Array(33-36)pkg/document/json/object.go (1)
Object(35-38)
🔇 Additional comments (9)
pkg/document/yson/yson.go (1)
486-491: Confirm semantics of implicit-type tree nodesAssigning the magic string
"root"whenever thetypefield is absent changes previous behaviour (which produced an empty string).
If the incoming node is actually a text node that simply omitted"type":"text"but does contain a"value"field, we will now mis-classify it and silently lose the value on re-marshal because non-text nodes ignoreValue.Please double-check:
- Do we have fixtures that rely on the old behaviour?
- Should we instead treat “has
value, but notype” as a text node?- At minimum, add a unit test that loads a node without
"type"and ensures it round-trips correctly.pkg/document/crdt/primitive.go (2)
19-21:encoding/base64import looks goodImporting
encoding/base64is necessary for the new bytes-to-string
conversion. No issues spotted.
259-261: Consider signalling that the value is base64-encoded
[]bytenow serialises as a plain JSON string containing the base64 text.
Down-stream consumers cannot distinguish this from an ordinary UTF-8 string.If interoperability matters you might wrap it in an object similar to the
YSONBinDatarepresentation, e.g.:{"type":"BinData","value":"AQID"}Alternatively prepend a prefix like
"base64::AQID".If keeping it as a raw string is intentional, please document the decision.
test/integration/object_test.go (6)
177-185: Great use of YSON literals in testsSwitching the fixture to
yson.ParseObjectmakes the intent clearer and
reduces boilerplate – nice!
234-245: Consistent literal usageSame comment as above – the tests read more naturally with YSON.
263-266: Nested array literal showcases parser flexibilityGood edge-case to keep in the suite.
288-291: Counters with YSON literals👍 This covers both
LongandIntvariants.
313-314: EmptyText()constructorThe empty-constructor path added in
preprocessTypeValuesis exercised here.
Nice coverage.
334-345: Full primitive-type matrixGood to see every primitive represented; this will catch regressions in the
pre-processor quickly.
hackerwins
force-pushed
the
literal-to-yson
branch
from
cef20c3 to
9e149ca
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
test/integration/yson_test.go (4)
129-129: Consider renaming this test for clarityThe test name mentions "json literal array" but it's actually testing YSON objects containing arrays.
- t.Run("object.set with json literal array type test", func(t *testing.T) { + t.Run("object.set with yson array type test", func(t *testing.T) {
49-56: Add error handling for YSON parsingThe code uses
yson.ParseObjectwhich panics on parsing errors. Consider adding a test for error handling or validating input to prevent potential runtime panics in production code.You could add a test case with invalid YSON to verify it's handled gracefully, or ensure the production code using this functionality has proper error handling.
149-150: Verify garbage collection count explanationThe comment mentions "3 elements: array, 1, 2" but doesn't explain why element "3" isn't counted. Consider adding a clearer explanation of the garbage collection expectations.
- // 03. garbage collect (3 elements: array, 1, 2) + // 03. garbage collect (3 elements: nested array at index 3 and its children [7, 8])
214-214: Consider adding timezone tests for Date typeThe test uses a UTC timestamp for Date, but consider adding tests with different timezones to ensure proper parsing and formatting across time zones.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
pkg/document/crdt/primitive.go(2 hunks)pkg/document/json/object.go(1 hunks)pkg/document/yson/yson.go(3 hunks)test/integration/gc_test.go(2 hunks)test/integration/object_test.go(0 hunks)test/integration/yson_test.go(1 hunks)
💤 Files with no reviewable changes (1)
- test/integration/object_test.go
🚧 Files skipped from review as they are similar to previous changes (4)
- pkg/document/crdt/primitive.go
- pkg/document/json/object.go
- pkg/document/yson/yson.go
- test/integration/gc_test.go
🧰 Additional context used
🧬 Code Graph Analysis (1)
test/integration/yson_test.go (4)
test/helper/helper.go (2)
TestDocKey(332-361)MaxVersionVector(154-165)pkg/document/json/object.go (1)
Object(35-38)pkg/document/yson/yson.go (2)
Object(74-74)ParseObject(565-571)pkg/document/crdt/counter.go (1)
LongCnt(37-37)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: complex-test
- GitHub Check: build
- GitHub Check: bench
🔇 Additional comments (3)
test/integration/yson_test.go (3)
35-221: Overall comprehensive test coverage for YSON operationsThis new integration test file provides excellent coverage for YSON functionality across different data types and operations. The tests validate parsing, synchronization, and garbage collection behaviors effectively.
205-206: Address TODO about long type precision lossThe comment indicates a known issue with precision loss when parsing long integer types through JSON unmarshaling. This could lead to data corruption for integers larger than 2^53.
Is there an issue tracking this TODO? Consider implementing a custom unmarshaler for the Long type to preserve precision.
91-94: Good validation of garbage collectionThe test effectively validates that all elements are correctly garbage collected after deletion, ensuring no memory leaks in the CRDT implementation.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
pkg/document/json/object.go (3)
104-109: Good addition of error handlingAdding error handling when creating a new counter improves code robustness. However, consider propagating the error instead of panicking, which would allow calling code to handle the error gracefully.
-func (p *Object) SetNewCounter(k string, t crdt.CounterType, n any) *Counter { - v := p.setInternal(k, func(ticket *time.Ticket) crdt.Element { - crdtCounter, err := crdt.NewCounter(t, n, ticket) - if err != nil { - panic(err) - } - - return NewCounter(n, t).Initialize(p.context, crdtCounter) - }) - - return v.(*Counter) +func (p *Object) SetNewCounter(k string, t crdt.CounterType, n any) (*Counter, error) { + var counterErr error + v := p.setInternal(k, func(ticket *time.Ticket) crdt.Element { + crdtCounter, err := crdt.NewCounter(t, n, ticket) + if err != nil { + counterErr = err + // Return a nil element or placeholder that will be handled below + return nil + } + + return NewCounter(n, t).Initialize(p.context, crdtCounter) + }) + + if counterErr != nil { + return nil, counterErr + } + return v.(*Counter), nil }
238-243: Verify YSON array setting in SetYSONElementThe
SetYSONElementmethod contains a potential inconsistency: when setting ayson.Array, it creates a new empty array and then populates it, whereas the updatedSetNewArraymethod can accept ayson.Arraydirectly for initialization. Consider updating this to use the new method signature:case yson.Array: - arr := p.SetNewArray(k) - for _, elem := range y { - arr.AddYSON(elem) - } + p.SetNewArray(k, y)
243-248: Similar inconsistency in Object handlingSimilar to the array case, the Object handling in
SetYSONElementcould be updated to use the new initialization pattern:case yson.Object: - o := p.SetNewObject(k) - for key, value := range y { - o.SetYSONElement(key, value) - } + p.SetNewObject(k, y)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
client/client.go(1 hunks)client/options.go(3 hunks)pkg/document/json/array.go(1 hunks)pkg/document/json/json.go(0 hunks)pkg/document/json/object.go(2 hunks)test/integration/document_test.go(7 hunks)
💤 Files with no reviewable changes (1)
- pkg/document/json/json.go
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: complex-test
- GitHub Check: bench
- GitHub Check: build
🔇 Additional comments (19)
client/client.go (1)
357-361: Code change improves type safety by using YSON elementsThe condition has been inverted and the code now uses
SetYSONElementinstead of the previousSetDynamicValue. This change aligns with the broader refactor to use YSON-typed objects instead of generic Go types, which improves type safety and reduces reliance on reflection.client/options.go (3)
25-25: Added import for YSON packageAdded import for the YSON package to support the type changes in this file.
97-97: Type change from map[string]any to yson.Object for stronger typingThe
InitialRootfield type has been changed from a genericmap[string]anyto a more specificyson.Object. This change improves type safety and aligns with the refactoring goal of replacing literals with YSON.
109-109: Updated parameter type to use yson.ObjectThe
WithInitialRootfunction now accepts ayson.Objectinstead of a genericmap[string]any, which complements the type change in theAttachOptionsstruct.pkg/document/json/array.go (1)
250-251: Simplified code by removing reflection-based type handlingThe removal of code here is part of a broader refactoring to eliminate reflection-based dynamic type handling. The
buildArrayElementsandsliceToElementsfunctions that previously existed here have been removed in favor of explicit YSON type handling.test/integration/document_test.go (9)
846-851: Replaced map literals with YSON object parsing for improved type safetyThe test code now uses
yson.ParseObjectwith a YSON-formatted string instead of map literals. This approach provides clearer type semantics and maps directly to the document model's new YSON-based implementation.
858-864: Consistent use of YSON parsing instead of map literalsThe change to use YSON strings provides a more standardized way to create test objects with special types like
Counterthat are properly typed in the YSON model.
874-879: Using YSON parsing maintains consistent pattern across testsThe update to use YSON parsing strings maintains a consistent pattern across all test cases where complex document objects need to be created.
897-902: Replaced map literals with parsed YSON for consistent testing approachUsing parsed YSON strings instead of map literals creates better alignment with how documents are stored internally after the refactoring.
912-914: Simple YSON parsing for string valuesEven for simple string values, the code consistently uses YSON parsing to maintain the new pattern throughout the codebase.
920-922: Consistent YSON parsing applicationThe code consistently applies the YSON parsing approach to create objects throughout all test cases.
943-951: Testing duplicate keys in YSON objectsThis test case verifies that when the same key appears multiple times in a YSON object, the last value is used. The change to use YSON parsing is consistent with the rest of the changes.
962-964: Using specialized YSON Counter typeThe test now uses YSON's
Counter(Long(1))to create a counter object with proper typing, which is more explicit about the expected type than the previous implementation.
970-972: Using YSON Text constructorThe test now uses YSON's
Text()constructor for creating text elements, which provides clearer semantics about the element type being created.pkg/document/json/object.go (5)
57-72: Improved type safety and initialization patternThe refactoring of
SetNewObjectto accept a variadic parameter ofyson.Objecttype is a good improvement. This change:
- Makes the API more type-safe by using YSON types directly instead of dynamic values
- Provides a cleaner way to initialize objects with predefined values
- Simplifies the code by removing reflection-based type conversion
74-89: Well-structured array initialization with YSONThe updated
SetNewArraymethod provides a consistent pattern matching the object initialization approach. This maintains API consistency across the library and improves type safety. The code now clearly shows how arrays are initialized and populated with initial values.
61-62: Simplified object creationThe code now creates objects in a cleaner way, using just the necessary CRDT elements. This aligns with the PR's goal of replacing literals with YSON types.
65-69: Consistent initialization patternThe conditional initialization of the object when YSON data is provided follows a clean pattern that matches the array initialization. This provides a consistent approach throughout the codebase.
82-86: Consistent array populationThe conditional initialization logic for arrays follows the same pattern as objects, maintaining consistency across the API. This provides a predictable interface for developers.
hackerwins
force-pushed
the
literal-to-yson
branch
from
2694e7c to
0026ee9
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
pkg/document/json/object.go (1)
104-110: Explicit error handling in SetNewCounter.The code now explicitly checks for errors when creating a new counter, which is a good practice. However, it still uses panic for error handling instead of returning an error.
While this is consistent with the rest of the codebase's error handling approach, consider transitioning to returning errors instead of panicking in a future refactoring.
Consider returning errors instead of panicking for better error handling:
-func (p *Object) SetNewCounter(k string, t crdt.CounterType, n any) *Counter { +func (p *Object) SetNewCounter(k string, t crdt.CounterType, n any) (*Counter, error) { v := p.setInternal(k, func(ticket *time.Ticket) crdt.Element { crdtCounter, err := crdt.NewCounter(t, n, ticket) if err != nil { - panic(err) + return nil, err } return NewCounter(n, t).Initialize(p.context, crdtCounter) }) - return v.(*Counter) + return v.(*Counter), nil }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
client/client.go(1 hunks)client/options.go(3 hunks)pkg/document/json/array.go(1 hunks)pkg/document/json/json.go(0 hunks)pkg/document/json/object.go(2 hunks)test/integration/document_test.go(7 hunks)test/integration/event_webhook_test.go(7 hunks)
💤 Files with no reviewable changes (1)
- pkg/document/json/json.go
✅ Files skipped from review due to trivial changes (1)
- test/integration/event_webhook_test.go
🚧 Files skipped from review as they are similar to previous changes (3)
- pkg/document/json/array.go
- client/options.go
- client/client.go
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: build
- GitHub Check: complex-test
- GitHub Check: bench
🔇 Additional comments (10)
test/integration/document_test.go (8)
38-38: Good addition of the yson package import.Adding the yson package import is necessary for the YSON parsing functionality used throughout the test file.
846-851: Improved type safety with YSON format.The change from Go map literals to explicit YSON format using
yson.ParseObjectprovides better type safety and aligns with the PR's goal of replacing literals with YSON. The YSON representation clearly indicates types (Counter, Int) which was implicit in the previous implementation.
858-864: LGTM - Consistent YSON representation of structured data.This change maintains the same test logic while switching to the new YSON format. The nested structure is preserved and correctly represents the same test data.
874-879: Consistent refactoring to YSON format.The consistent application of YSON format across all test cases helps standardize how document data is represented and initialized.
897-902: Maintains test logic while improving type clarity.The test case correctly preserves its functionality while making the types explicit in the YSON format.
912-914: Standardized approach to initializing document roots.Both client attachments consistently use the same YSON approach, making the test easier to understand and maintain.
Also applies to: 920-922
943-951: Good test for multiple same-key entries in YSON.This test case verifies the expected behavior when the same key appears multiple times in a YSON object (should use the last value). The refactoring preserves this important test case.
962-964: Type conflict test properly implemented with YSON.The test case for type conflicts is properly preserved in the YSON format, ensuring that the system correctly handles type conflicts between Counter and Text types.
Also applies to: 970-972
pkg/document/json/object.go (2)
57-72: Improved API with direct YSON initialization support.The
SetNewObjectmethod has been enhanced to accept an optional YSON object for initialization, providing a more direct and type-safe way to initialize objects without relying on reflection. The documentation has been updated to reflect this change, and the implementation correctly handles the optional parameter.This approach is more efficient and explicit than the previous reflection-based approach that was likely removed.
74-89: Consistent pattern applied to SetNewArray.Similar to the changes in
SetNewObject, theSetNewArraymethod now accepts an optional YSON array for initialization. This maintains consistency in the API design and provides the same benefits of improved type safety and reduced reflection.
hackerwins
changed the title
2 participants
What this PR does / why we need it:
Replace literals with YSON
This commit refactors client attach logic and CRDT initialization to adopt YSON as the primary data representation format. It introduces functions for parsing YSON strings into arrays and objects, enabling consistent and structured initialization of document roots.
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