Optimize FindChangeInfosBetweenServerSeqs to prevent unnecessary Query #974
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
WalkthroughThe recent changes introduce guard clauses in the Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add 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
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- server/backend/database/mongo/client.go (1 hunks)
Additional comments not posted (1)
server/backend/database/mongo/client.go (1)
1033-1035: Optimization approved: Early return forfrom > to.The addition of the guard clause is a valid optimization to prevent unnecessary database queries when
fromis greater thanto. This change improves efficiency by avoiding redundant operations.Ensure that this early return does not affect any other logic that might depend on the method's execution when
from > to.Run the following script to verify if the early return impacts any other logic:
Verification successful
No impact from early return in
FindChangeInfosBetweenServerSeqs.The early return optimization in the
FindChangeInfosBetweenServerSeqsmethod does not affect other logic in the codebase. The method is used in a straightforward manner, and there are no dependencies on its execution whenfrom > to.
- Locations:
server/packs/pushpull.goserver/backend/database/memory/database.goScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify if the early return in `FindChangeInfosBetweenServerSeqs` affects other logic. # Test: Search for the method usage. Expect: No logic depends on execution when `from > to`. rg --type go -A 5 $'FindChangeInfosBetweenServerSeqs'Length of output: 2711
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #974 +/- ##
==========================================
+ Coverage 51.06% 51.14% +0.07%
==========================================
Files 73 73
Lines 10782 10788 +6
==========================================
+ Hits 5506 5517 +11
+ Misses 4725 4722 -3
+ Partials 551 549 -2 ☔ View full report in Codecov by Sentry. |
hackerwins
reviewed
Aug 22, 2024
There was a problem hiding this comment.
Thanks for your contribution.
Can you leave the problem case described in the issue as a test case?
https://github.com/yorkie-team/yorkie/blob/main/server/backend/database/testcases/testcases.go
There was a problem hiding this comment.
Actionable comments posted: 1
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (5)
- server/backend/database/memory/database.go (1 hunks)
- server/backend/database/memory/database_test.go (1 hunks)
- server/backend/database/mongo/client_test.go (1 hunks)
- server/backend/database/testcases/testcases.go (1 hunks)
- test/sharding/mongo_client_test.go (1 hunks)
Additional context used
golangci-lint
server/backend/database/testcases/testcases.go
362-362: ineffectual assignment to err
(ineffassign)
GitHub Check: build
server/backend/database/testcases/testcases.go
[failure] 362-362:
ineffectual assignment to err (ineffassign)
Additional comments not posted (5)
server/backend/database/memory/database_test.go (1)
63-65: New test case added forRunFindChangeInfosBetweenServerSeqsTest.The test case is correctly added and follows the existing structure of the test suite.
server/backend/database/mongo/client_test.go (1)
79-81: New test case added forRunFindChangeInfosBetweenServerSeqsTest.The test case is correctly added and follows the existing structure of the test suite.
test/sharding/mongo_client_test.go (1)
88-90: New test case added forRunFindChangeInfosBetweenServerSeqsTest.The test case is correctly added and follows the existing structure of the test suite.
server/backend/database/memory/database.go (1)
1054-1056: Guard clause to prevent unnecessary queries.The newly added guard clause effectively prevents the function from executing unnecessary database operations when
fromis greater thanto. This optimization aligns with the PR's objective to reduce database load and enhance performance.server/backend/database/testcases/testcases.go (1)
346-380: LGTM! The test is well-structured and covers the intended scenario.The function effectively tests the behavior of
FindChangeInfosBetweenServerSeqswhen no interference from other users occurs.Tools
golangci-lint
362-362: ineffectual assignment to err
(ineffassign)
GitHub Check: build
[failure] 362-362:
ineffectual assignment to err (ineffassign)
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Go Benchmark
| Benchmark suite | Current: 1f7dbd3 | Previous: d30abf3 | Ratio |
|---|---|---|---|
BenchmarkDocument/constructor_test |
1501 ns/op 1337 B/op 24 allocs/op |
1492 ns/op 1337 B/op 24 allocs/op |
1.01 |
BenchmarkDocument/constructor_test - ns/op |
1501 ns/op |
1492 ns/op |
1.01 |
BenchmarkDocument/constructor_test - B/op |
1337 B/op |
1337 B/op |
1 |
BenchmarkDocument/constructor_test - allocs/op |
24 allocs/op |
24 allocs/op |
1 |
BenchmarkDocument/status_test |
1089 ns/op 1305 B/op 22 allocs/op |
1089 ns/op 1305 B/op 22 allocs/op |
1 |
BenchmarkDocument/status_test - ns/op |
1089 ns/op |
1089 ns/op |
1 |
BenchmarkDocument/status_test - B/op |
1305 B/op |
1305 B/op |
1 |
BenchmarkDocument/status_test - allocs/op |
22 allocs/op |
22 allocs/op |
1 |
BenchmarkDocument/equals_test |
7630 ns/op 7273 B/op 132 allocs/op |
7573 ns/op 7273 B/op 132 allocs/op |
1.01 |
BenchmarkDocument/equals_test - ns/op |
7630 ns/op |
7573 ns/op |
1.01 |
BenchmarkDocument/equals_test - B/op |
7273 B/op |
7273 B/op |
1 |
BenchmarkDocument/equals_test - allocs/op |
132 allocs/op |
132 allocs/op |
1 |
BenchmarkDocument/nested_update_test |
16922 ns/op 12139 B/op 262 allocs/op |
16776 ns/op 12138 B/op 262 allocs/op |
1.01 |
BenchmarkDocument/nested_update_test - ns/op |
16922 ns/op |
16776 ns/op |
1.01 |
BenchmarkDocument/nested_update_test - B/op |
12139 B/op |
12138 B/op |
1.00 |
BenchmarkDocument/nested_update_test - allocs/op |
262 allocs/op |
262 allocs/op |
1 |
BenchmarkDocument/delete_test |
22666 ns/op 15364 B/op 341 allocs/op |
22877 ns/op 15363 B/op 341 allocs/op |
0.99 |
BenchmarkDocument/delete_test - ns/op |
22666 ns/op |
22877 ns/op |
0.99 |
BenchmarkDocument/delete_test - B/op |
15364 B/op |
15363 B/op |
1.00 |
BenchmarkDocument/delete_test - allocs/op |
341 allocs/op |
341 allocs/op |
1 |
BenchmarkDocument/object_test |
8576 ns/op 6817 B/op 120 allocs/op |
8671 ns/op 6817 B/op 120 allocs/op |
0.99 |
BenchmarkDocument/object_test - ns/op |
8576 ns/op |
8671 ns/op |
0.99 |
BenchmarkDocument/object_test - B/op |
6817 B/op |
6817 B/op |
1 |
BenchmarkDocument/object_test - allocs/op |
120 allocs/op |
120 allocs/op |
1 |
BenchmarkDocument/array_test |
29110 ns/op 11947 B/op 276 allocs/op |
29059 ns/op 11947 B/op 276 allocs/op |
1.00 |
BenchmarkDocument/array_test - ns/op |
29110 ns/op |
29059 ns/op |
1.00 |
BenchmarkDocument/array_test - B/op |
11947 B/op |
11947 B/op |
1 |
BenchmarkDocument/array_test - allocs/op |
276 allocs/op |
276 allocs/op |
1 |
BenchmarkDocument/text_test |
30385 ns/op 14715 B/op 469 allocs/op |
30488 ns/op 14715 B/op 469 allocs/op |
1.00 |
BenchmarkDocument/text_test - ns/op |
30385 ns/op |
30488 ns/op |
1.00 |
BenchmarkDocument/text_test - B/op |
14715 B/op |
14715 B/op |
1 |
BenchmarkDocument/text_test - allocs/op |
469 allocs/op |
469 allocs/op |
1 |
BenchmarkDocument/text_composition_test |
28836 ns/op 18422 B/op 484 allocs/op |
28688 ns/op 18422 B/op 484 allocs/op |
1.01 |
BenchmarkDocument/text_composition_test - ns/op |
28836 ns/op |
28688 ns/op |
1.01 |
BenchmarkDocument/text_composition_test - B/op |
18422 B/op |
18422 B/op |
1 |
BenchmarkDocument/text_composition_test - allocs/op |
484 allocs/op |
484 allocs/op |
1 |
BenchmarkDocument/rich_text_test |
80181 ns/op 38475 B/op 1148 allocs/op |
80269 ns/op 38476 B/op 1148 allocs/op |
1.00 |
BenchmarkDocument/rich_text_test - ns/op |
80181 ns/op |
80269 ns/op |
1.00 |
BenchmarkDocument/rich_text_test - B/op |
38475 B/op |
38476 B/op |
1.00 |
BenchmarkDocument/rich_text_test - allocs/op |
1148 allocs/op |
1148 allocs/op |
1 |
BenchmarkDocument/counter_test |
17346 ns/op 10722 B/op 244 allocs/op |
17423 ns/op 10722 B/op 244 allocs/op |
1.00 |
BenchmarkDocument/counter_test - ns/op |
17346 ns/op |
17423 ns/op |
1.00 |
BenchmarkDocument/counter_test - B/op |
10722 B/op |
10722 B/op |
1 |
BenchmarkDocument/counter_test - allocs/op |
244 allocs/op |
244 allocs/op |
1 |
BenchmarkDocument/text_edit_gc_100 |
1275626 ns/op 870915 B/op 16752 allocs/op |
1272128 ns/op 870923 B/op 16752 allocs/op |
1.00 |
BenchmarkDocument/text_edit_gc_100 - ns/op |
1275626 ns/op |
1272128 ns/op |
1.00 |
BenchmarkDocument/text_edit_gc_100 - B/op |
870915 B/op |
870923 B/op |
1.00 |
BenchmarkDocument/text_edit_gc_100 - allocs/op |
16752 allocs/op |
16752 allocs/op |
1 |
BenchmarkDocument/text_edit_gc_1000 |
48937398 ns/op 50536264 B/op 181715 allocs/op |
50403985 ns/op 50536255 B/op 181712 allocs/op |
0.97 |
BenchmarkDocument/text_edit_gc_1000 - ns/op |
48937398 ns/op |
50403985 ns/op |
0.97 |
BenchmarkDocument/text_edit_gc_1000 - B/op |
50536264 B/op |
50536255 B/op |
1.00 |
BenchmarkDocument/text_edit_gc_1000 - allocs/op |
181715 allocs/op |
181712 allocs/op |
1.00 |
BenchmarkDocument/text_split_gc_100 |
1854559 ns/op 1528817 B/op 15605 allocs/op |
1861041 ns/op 1528791 B/op 15604 allocs/op |
1.00 |
BenchmarkDocument/text_split_gc_100 - ns/op |
1854559 ns/op |
1861041 ns/op |
1.00 |
BenchmarkDocument/text_split_gc_100 - B/op |
1528817 B/op |
1528791 B/op |
1.00 |
BenchmarkDocument/text_split_gc_100 - allocs/op |
15605 allocs/op |
15604 allocs/op |
1.00 |
BenchmarkDocument/text_split_gc_1000 |
110181013 ns/op 135077348 B/op 182202 allocs/op |
111569335 ns/op 135075657 B/op 182189 allocs/op |
0.99 |
BenchmarkDocument/text_split_gc_1000 - ns/op |
110181013 ns/op |
111569335 ns/op |
0.99 |
BenchmarkDocument/text_split_gc_1000 - B/op |
135077348 B/op |
135075657 B/op |
1.00 |
BenchmarkDocument/text_split_gc_1000 - allocs/op |
182202 allocs/op |
182189 allocs/op |
1.00 |
BenchmarkDocument/text_delete_all_10000 |
15074311 ns/op 10185894 B/op 40681 allocs/op |
15659641 ns/op 10182929 B/op 40676 allocs/op |
0.96 |
BenchmarkDocument/text_delete_all_10000 - ns/op |
15074311 ns/op |
15659641 ns/op |
0.96 |
BenchmarkDocument/text_delete_all_10000 - B/op |
10185894 B/op |
10182929 B/op |
1.00 |
BenchmarkDocument/text_delete_all_10000 - allocs/op |
40681 allocs/op |
40676 allocs/op |
1.00 |
BenchmarkDocument/text_delete_all_100000 |
283100742 ns/op 142669724 B/op 411677 allocs/op |
283342119 ns/op 142687568 B/op 411725 allocs/op |
1.00 |
BenchmarkDocument/text_delete_all_100000 - ns/op |
283100742 ns/op |
283342119 ns/op |
1.00 |
BenchmarkDocument/text_delete_all_100000 - B/op |
142669724 B/op |
142687568 B/op |
1.00 |
BenchmarkDocument/text_delete_all_100000 - allocs/op |
411677 allocs/op |
411725 allocs/op |
1.00 |
BenchmarkDocument/text_100 |
217157 ns/op 120037 B/op 5081 allocs/op |
226783 ns/op 120037 B/op 5081 allocs/op |
0.96 |
BenchmarkDocument/text_100 - ns/op |
217157 ns/op |
226783 ns/op |
0.96 |
BenchmarkDocument/text_100 - B/op |
120037 B/op |
120037 B/op |
1 |
BenchmarkDocument/text_100 - allocs/op |
5081 allocs/op |
5081 allocs/op |
1 |
BenchmarkDocument/text_1000 |
2336935 ns/op 1169022 B/op 50085 allocs/op |
2444703 ns/op 1169027 B/op 50085 allocs/op |
0.96 |
BenchmarkDocument/text_1000 - ns/op |
2336935 ns/op |
2444703 ns/op |
0.96 |
BenchmarkDocument/text_1000 - B/op |
1169022 B/op |
1169027 B/op |
1.00 |
BenchmarkDocument/text_1000 - allocs/op |
50085 allocs/op |
50085 allocs/op |
1 |
BenchmarkDocument/array_1000 |
1196686 ns/op 1091364 B/op 11831 allocs/op |
1274460 ns/op 1091425 B/op 11832 allocs/op |
0.94 |
BenchmarkDocument/array_1000 - ns/op |
1196686 ns/op |
1274460 ns/op |
0.94 |
BenchmarkDocument/array_1000 - B/op |
1091364 B/op |
1091425 B/op |
1.00 |
BenchmarkDocument/array_1000 - allocs/op |
11831 allocs/op |
11832 allocs/op |
1.00 |
BenchmarkDocument/array_10000 |
13203233 ns/op 9800337 B/op 120297 allocs/op |
13088473 ns/op 9800002 B/op 120296 allocs/op |
1.01 |
BenchmarkDocument/array_10000 - ns/op |
13203233 ns/op |
13088473 ns/op |
1.01 |
BenchmarkDocument/array_10000 - B/op |
9800337 B/op |
9800002 B/op |
1.00 |
BenchmarkDocument/array_10000 - allocs/op |
120297 allocs/op |
120296 allocs/op |
1.00 |
BenchmarkDocument/array_gc_100 |
145433 ns/op 132720 B/op 1260 allocs/op |
154144 ns/op 132725 B/op 1261 allocs/op |
0.94 |
BenchmarkDocument/array_gc_100 - ns/op |
145433 ns/op |
154144 ns/op |
0.94 |
BenchmarkDocument/array_gc_100 - B/op |
132720 B/op |
132725 B/op |
1.00 |
BenchmarkDocument/array_gc_100 - allocs/op |
1260 allocs/op |
1261 allocs/op |
1.00 |
BenchmarkDocument/array_gc_1000 |
1373694 ns/op 1159219 B/op 12877 allocs/op |
1450380 ns/op 1159135 B/op 12876 allocs/op |
0.95 |
BenchmarkDocument/array_gc_1000 - ns/op |
1373694 ns/op |
1450380 ns/op |
0.95 |
BenchmarkDocument/array_gc_1000 - B/op |
1159219 B/op |
1159135 B/op |
1.00 |
BenchmarkDocument/array_gc_1000 - allocs/op |
12877 allocs/op |
12876 allocs/op |
1.00 |
BenchmarkDocument/counter_1000 |
197418 ns/op 193081 B/op 5771 allocs/op |
210590 ns/op 193080 B/op 5771 allocs/op |
0.94 |
BenchmarkDocument/counter_1000 - ns/op |
197418 ns/op |
210590 ns/op |
0.94 |
BenchmarkDocument/counter_1000 - B/op |
193081 B/op |
193080 B/op |
1.00 |
BenchmarkDocument/counter_1000 - allocs/op |
5771 allocs/op |
5771 allocs/op |
1 |
BenchmarkDocument/counter_10000 |
2146990 ns/op 2088013 B/op 59778 allocs/op |
2219584 ns/op 2087998 B/op 59778 allocs/op |
0.97 |
BenchmarkDocument/counter_10000 - ns/op |
2146990 ns/op |
2219584 ns/op |
0.97 |
BenchmarkDocument/counter_10000 - B/op |
2088013 B/op |
2087998 B/op |
1.00 |
BenchmarkDocument/counter_10000 - allocs/op |
59778 allocs/op |
59778 allocs/op |
1 |
BenchmarkDocument/object_1000 |
1363980 ns/op 1428347 B/op 9849 allocs/op |
1461379 ns/op 1428004 B/op 9849 allocs/op |
0.93 |
BenchmarkDocument/object_1000 - ns/op |
1363980 ns/op |
1461379 ns/op |
0.93 |
BenchmarkDocument/object_1000 - B/op |
1428347 B/op |
1428004 B/op |
1.00 |
BenchmarkDocument/object_1000 - allocs/op |
9849 allocs/op |
9849 allocs/op |
1 |
BenchmarkDocument/object_10000 |
15483503 ns/op 12167750 B/op 100568 allocs/op |
15260717 ns/op 12168434 B/op 100568 allocs/op |
1.01 |
BenchmarkDocument/object_10000 - ns/op |
15483503 ns/op |
15260717 ns/op |
1.01 |
BenchmarkDocument/object_10000 - B/op |
12167750 B/op |
12168434 B/op |
1.00 |
BenchmarkDocument/object_10000 - allocs/op |
100568 allocs/op |
100568 allocs/op |
1 |
BenchmarkDocument/tree_100 |
1055495 ns/op 943703 B/op 6101 allocs/op |
1067159 ns/op 943702 B/op 6101 allocs/op |
0.99 |
BenchmarkDocument/tree_100 - ns/op |
1055495 ns/op |
1067159 ns/op |
0.99 |
BenchmarkDocument/tree_100 - B/op |
943703 B/op |
943702 B/op |
1.00 |
BenchmarkDocument/tree_100 - allocs/op |
6101 allocs/op |
6101 allocs/op |
1 |
BenchmarkDocument/tree_1000 |
76465298 ns/op 86460376 B/op 60115 allocs/op |
79864260 ns/op 86460427 B/op 60114 allocs/op |
0.96 |
BenchmarkDocument/tree_1000 - ns/op |
76465298 ns/op |
79864260 ns/op |
0.96 |
BenchmarkDocument/tree_1000 - B/op |
86460376 B/op |
86460427 B/op |
1.00 |
BenchmarkDocument/tree_1000 - allocs/op |
60115 allocs/op |
60114 allocs/op |
1.00 |
BenchmarkDocument/tree_10000 |
9279135054 ns/op 8580669136 B/op 600214 allocs/op |
9773338423 ns/op 8580652464 B/op 600220 allocs/op |
0.95 |
BenchmarkDocument/tree_10000 - ns/op |
9279135054 ns/op |
9773338423 ns/op |
0.95 |
BenchmarkDocument/tree_10000 - B/op |
8580669136 B/op |
8580652464 B/op |
1.00 |
BenchmarkDocument/tree_10000 - allocs/op |
600214 allocs/op |
600220 allocs/op |
1.00 |
BenchmarkDocument/tree_delete_all_1000 |
72804631 ns/op 87509808 B/op 75265 allocs/op |
80566958 ns/op 87509565 B/op 75265 allocs/op |
0.90 |
BenchmarkDocument/tree_delete_all_1000 - ns/op |
72804631 ns/op |
80566958 ns/op |
0.90 |
BenchmarkDocument/tree_delete_all_1000 - B/op |
87509808 B/op |
87509565 B/op |
1.00 |
BenchmarkDocument/tree_delete_all_1000 - allocs/op |
75265 allocs/op |
75265 allocs/op |
1 |
BenchmarkDocument/tree_edit_gc_100 |
3706168 ns/op 4147675 B/op 15140 allocs/op |
3959142 ns/op 4147722 B/op 15141 allocs/op |
0.94 |
BenchmarkDocument/tree_edit_gc_100 - ns/op |
3706168 ns/op |
3959142 ns/op |
0.94 |
BenchmarkDocument/tree_edit_gc_100 - B/op |
4147675 B/op |
4147722 B/op |
1.00 |
BenchmarkDocument/tree_edit_gc_100 - allocs/op |
15140 allocs/op |
15141 allocs/op |
1.00 |
BenchmarkDocument/tree_edit_gc_1000 |
291455259 ns/op 383745728 B/op 154839 allocs/op |
319781842 ns/op 383745122 B/op 154855 allocs/op |
0.91 |
BenchmarkDocument/tree_edit_gc_1000 - ns/op |
291455259 ns/op |
319781842 ns/op |
0.91 |
BenchmarkDocument/tree_edit_gc_1000 - B/op |
383745728 B/op |
383745122 B/op |
1.00 |
BenchmarkDocument/tree_edit_gc_1000 - allocs/op |
154839 allocs/op |
154855 allocs/op |
1.00 |
BenchmarkDocument/tree_split_gc_100 |
2465624 ns/op 2412428 B/op 11125 allocs/op |
2654848 ns/op 2412533 B/op 11125 allocs/op |
0.93 |
BenchmarkDocument/tree_split_gc_100 - ns/op |
2465624 ns/op |
2654848 ns/op |
0.93 |
BenchmarkDocument/tree_split_gc_100 - B/op |
2412428 B/op |
2412533 B/op |
1.00 |
BenchmarkDocument/tree_split_gc_100 - allocs/op |
11125 allocs/op |
11125 allocs/op |
1 |
BenchmarkDocument/tree_split_gc_1000 |
178233804 ns/op 222249466 B/op 121985 allocs/op |
195735721 ns/op 222249554 B/op 121995 allocs/op |
0.91 |
BenchmarkDocument/tree_split_gc_1000 - ns/op |
178233804 ns/op |
195735721 ns/op |
0.91 |
BenchmarkDocument/tree_split_gc_1000 - B/op |
222249466 B/op |
222249554 B/op |
1.00 |
BenchmarkDocument/tree_split_gc_1000 - allocs/op |
121985 allocs/op |
121995 allocs/op |
1.00 |
BenchmarkRPC/client_to_server |
341599961 ns/op 16283234 B/op 167383 allocs/op |
341850453 ns/op 16320928 B/op 167395 allocs/op |
1.00 |
BenchmarkRPC/client_to_server - ns/op |
341599961 ns/op |
341850453 ns/op |
1.00 |
BenchmarkRPC/client_to_server - B/op |
16283234 B/op |
16320928 B/op |
1.00 |
BenchmarkRPC/client_to_server - allocs/op |
167383 allocs/op |
167395 allocs/op |
1.00 |
BenchmarkRPC/client_to_client_via_server |
628594591 ns/op 35538804 B/op 321481 allocs/op |
626359912 ns/op 32271528 B/op 321208 allocs/op |
1.00 |
BenchmarkRPC/client_to_client_via_server - ns/op |
628594591 ns/op |
626359912 ns/op |
1.00 |
BenchmarkRPC/client_to_client_via_server - B/op |
35538804 B/op |
32271528 B/op |
1.10 |
BenchmarkRPC/client_to_client_via_server - allocs/op |
321481 allocs/op |
321208 allocs/op |
1.00 |
BenchmarkRPC/attach_large_document |
1324386320 ns/op 1908604872 B/op 8776 allocs/op |
1391601896 ns/op 1895485064 B/op 8756 allocs/op |
0.95 |
BenchmarkRPC/attach_large_document - ns/op |
1324386320 ns/op |
1391601896 ns/op |
0.95 |
BenchmarkRPC/attach_large_document - B/op |
1908604872 B/op |
1895485064 B/op |
1.01 |
BenchmarkRPC/attach_large_document - allocs/op |
8776 allocs/op |
8756 allocs/op |
1.00 |
BenchmarkRPC/adminCli_to_server |
547510424 ns/op 36773768 B/op 289562 allocs/op |
535911068 ns/op 37181220 B/op 289605 allocs/op |
1.02 |
BenchmarkRPC/adminCli_to_server - ns/op |
547510424 ns/op |
535911068 ns/op |
1.02 |
BenchmarkRPC/adminCli_to_server - B/op |
36773768 B/op |
37181220 B/op |
0.99 |
BenchmarkRPC/adminCli_to_server - allocs/op |
289562 allocs/op |
289605 allocs/op |
1.00 |
BenchmarkLocker |
63.91 ns/op 16 B/op 1 allocs/op |
64.1 ns/op 16 B/op 1 allocs/op |
1.00 |
BenchmarkLocker - ns/op |
63.91 ns/op |
64.1 ns/op |
1.00 |
BenchmarkLocker - B/op |
16 B/op |
16 B/op |
1 |
BenchmarkLocker - allocs/op |
1 allocs/op |
1 allocs/op |
1 |
BenchmarkLockerParallel |
38.86 ns/op 0 B/op 0 allocs/op |
39.09 ns/op 0 B/op 0 allocs/op |
0.99 |
BenchmarkLockerParallel - ns/op |
38.86 ns/op |
39.09 ns/op |
0.99 |
BenchmarkLockerParallel - B/op |
0 B/op |
0 B/op |
1 |
BenchmarkLockerParallel - allocs/op |
0 allocs/op |
0 allocs/op |
1 |
BenchmarkLockerMoreKeys |
151.1 ns/op 15 B/op 0 allocs/op |
151.2 ns/op 15 B/op 0 allocs/op |
1.00 |
BenchmarkLockerMoreKeys - ns/op |
151.1 ns/op |
151.2 ns/op |
1.00 |
BenchmarkLockerMoreKeys - B/op |
15 B/op |
15 B/op |
1 |
BenchmarkLockerMoreKeys - allocs/op |
0 allocs/op |
0 allocs/op |
1 |
BenchmarkChange/Push_10_Changes |
3591487 ns/op 116486 B/op 1200 allocs/op |
3575420 ns/op 116682 B/op 1205 allocs/op |
1.00 |
BenchmarkChange/Push_10_Changes - ns/op |
3591487 ns/op |
3575420 ns/op |
1.00 |
BenchmarkChange/Push_10_Changes - B/op |
116486 B/op |
116682 B/op |
1.00 |
BenchmarkChange/Push_10_Changes - allocs/op |
1200 allocs/op |
1205 allocs/op |
1.00 |
BenchmarkChange/Push_100_Changes |
14273407 ns/op 569991 B/op 6571 allocs/op |
14099082 ns/op 572001 B/op 6577 allocs/op |
1.01 |
BenchmarkChange/Push_100_Changes - ns/op |
14273407 ns/op |
14099082 ns/op |
1.01 |
BenchmarkChange/Push_100_Changes - B/op |
569991 B/op |
572001 B/op |
1.00 |
BenchmarkChange/Push_100_Changes - allocs/op |
6571 allocs/op |
6577 allocs/op |
1.00 |
BenchmarkChange/Push_1000_Changes |
116261328 ns/op 5361213 B/op 63066 allocs/op |
115349593 ns/op 5346453 B/op 63070 allocs/op |
1.01 |
BenchmarkChange/Push_1000_Changes - ns/op |
116261328 ns/op |
115349593 ns/op |
1.01 |
BenchmarkChange/Push_1000_Changes - B/op |
5361213 B/op |
5346453 B/op |
1.00 |
BenchmarkChange/Push_1000_Changes - allocs/op |
63066 allocs/op |
63070 allocs/op |
1.00 |
BenchmarkChange/Pull_10_Changes |
2909694 ns/op 101575 B/op 1008 allocs/op |
2912412 ns/op 101746 B/op 1008 allocs/op |
1.00 |
BenchmarkChange/Pull_10_Changes - ns/op |
2909694 ns/op |
2912412 ns/op |
1.00 |
BenchmarkChange/Pull_10_Changes - B/op |
101575 B/op |
101746 B/op |
1.00 |
BenchmarkChange/Pull_10_Changes - allocs/op |
1008 allocs/op |
1008 allocs/op |
1 |
BenchmarkChange/Pull_100_Changes |
4382702 ns/op 267369 B/op 3479 allocs/op |
4369694 ns/op 268011 B/op 3479 allocs/op |
1.00 |
BenchmarkChange/Pull_100_Changes - ns/op |
4382702 ns/op |
4369694 ns/op |
1.00 |
BenchmarkChange/Pull_100_Changes - B/op |
267369 B/op |
268011 B/op |
1.00 |
BenchmarkChange/Pull_100_Changes - allocs/op |
3479 allocs/op |
3479 allocs/op |
1 |
BenchmarkChange/Pull_1000_Changes |
8638414 ns/op 1493297 B/op 29860 allocs/op |
8776228 ns/op 1491975 B/op 29863 allocs/op |
0.98 |
BenchmarkChange/Pull_1000_Changes - ns/op |
8638414 ns/op |
8776228 ns/op |
0.98 |
BenchmarkChange/Pull_1000_Changes - B/op |
1493297 B/op |
1491975 B/op |
1.00 |
BenchmarkChange/Pull_1000_Changes - allocs/op |
29860 allocs/op |
29863 allocs/op |
1.00 |
BenchmarkSnapshot/Push_3KB_snapshot |
16596485 ns/op 714254 B/op 6576 allocs/op |
16672527 ns/op 711822 B/op 6576 allocs/op |
1.00 |
BenchmarkSnapshot/Push_3KB_snapshot - ns/op |
16596485 ns/op |
16672527 ns/op |
1.00 |
BenchmarkSnapshot/Push_3KB_snapshot - B/op |
714254 B/op |
711822 B/op |
1.00 |
BenchmarkSnapshot/Push_3KB_snapshot - allocs/op |
6576 allocs/op |
6576 allocs/op |
1 |
BenchmarkSnapshot/Push_30KB_snapshot |
119793483 ns/op 5626699 B/op 63069 allocs/op |
118429396 ns/op 5640162 B/op 63070 allocs/op |
1.01 |
BenchmarkSnapshot/Push_30KB_snapshot - ns/op |
119793483 ns/op |
118429396 ns/op |
1.01 |
BenchmarkSnapshot/Push_30KB_snapshot - B/op |
5626699 B/op |
5640162 B/op |
1.00 |
BenchmarkSnapshot/Push_30KB_snapshot - allocs/op |
63069 allocs/op |
63070 allocs/op |
1.00 |
BenchmarkSnapshot/Pull_3KB_snapshot |
6451191 ns/op 923452 B/op 15514 allocs/op |
6434039 ns/op 924550 B/op 15515 allocs/op |
1.00 |
BenchmarkSnapshot/Pull_3KB_snapshot - ns/op |
6451191 ns/op |
6434039 ns/op |
1.00 |
BenchmarkSnapshot/Pull_3KB_snapshot - B/op |
923452 B/op |
924550 B/op |
1.00 |
BenchmarkSnapshot/Pull_3KB_snapshot - allocs/op |
15514 allocs/op |
15515 allocs/op |
1.00 |
BenchmarkSnapshot/Pull_30KB_snapshot |
15253090 ns/op 7160674 B/op 150112 allocs/op |
15184794 ns/op 7154732 B/op 150111 allocs/op |
1.00 |
BenchmarkSnapshot/Pull_30KB_snapshot - ns/op |
15253090 ns/op |
15184794 ns/op |
1.00 |
BenchmarkSnapshot/Pull_30KB_snapshot - B/op |
7160674 B/op |
7154732 B/op |
1.00 |
BenchmarkSnapshot/Pull_30KB_snapshot - allocs/op |
150112 allocs/op |
150111 allocs/op |
1.00 |
BenchmarkSync/memory_sync_10_test |
6919 ns/op 1286 B/op 38 allocs/op |
6726 ns/op 1286 B/op 38 allocs/op |
1.03 |
BenchmarkSync/memory_sync_10_test - ns/op |
6919 ns/op |
6726 ns/op |
1.03 |
BenchmarkSync/memory_sync_10_test - B/op |
1286 B/op |
1286 B/op |
1 |
BenchmarkSync/memory_sync_10_test - allocs/op |
38 allocs/op |
38 allocs/op |
1 |
BenchmarkSync/memory_sync_100_test |
50537 ns/op 8653 B/op 274 allocs/op |
50723 ns/op 8642 B/op 273 allocs/op |
1.00 |
BenchmarkSync/memory_sync_100_test - ns/op |
50537 ns/op |
50723 ns/op |
1.00 |
BenchmarkSync/memory_sync_100_test - B/op |
8653 B/op |
8642 B/op |
1.00 |
BenchmarkSync/memory_sync_100_test - allocs/op |
274 allocs/op |
273 allocs/op |
1.00 |
BenchmarkSync/memory_sync_1000_test |
582683 ns/op 74244 B/op 2114 allocs/op |
587984 ns/op 74020 B/op 2098 allocs/op |
0.99 |
BenchmarkSync/memory_sync_1000_test - ns/op |
582683 ns/op |
587984 ns/op |
0.99 |
BenchmarkSync/memory_sync_1000_test - B/op |
74244 B/op |
74020 B/op |
1.00 |
BenchmarkSync/memory_sync_1000_test - allocs/op |
2114 allocs/op |
2098 allocs/op |
1.01 |
BenchmarkSync/memory_sync_10000_test |
6948545 ns/op 738827 B/op 20324 allocs/op |
7194170 ns/op 740562 B/op 20289 allocs/op |
0.97 |
BenchmarkSync/memory_sync_10000_test - ns/op |
6948545 ns/op |
7194170 ns/op |
0.97 |
BenchmarkSync/memory_sync_10000_test - B/op |
738827 B/op |
740562 B/op |
1.00 |
BenchmarkSync/memory_sync_10000_test - allocs/op |
20324 allocs/op |
20289 allocs/op |
1.00 |
BenchmarkTextEditing |
5206214779 ns/op 3901940480 B/op 18743256 allocs/op |
5033459218 ns/op 3901913312 B/op 18743192 allocs/op |
1.03 |
BenchmarkTextEditing - ns/op |
5206214779 ns/op |
5033459218 ns/op |
1.03 |
BenchmarkTextEditing - B/op |
3901940480 B/op |
3901913312 B/op |
1.00 |
BenchmarkTextEditing - allocs/op |
18743256 allocs/op |
18743192 allocs/op |
1.00 |
BenchmarkTree/10000_vertices_to_protobuf |
3430046 ns/op 6262993 B/op 70025 allocs/op |
3394218 ns/op 6262992 B/op 70025 allocs/op |
1.01 |
BenchmarkTree/10000_vertices_to_protobuf - ns/op |
3430046 ns/op |
3394218 ns/op |
1.01 |
BenchmarkTree/10000_vertices_to_protobuf - B/op |
6262993 B/op |
6262992 B/op |
1.00 |
BenchmarkTree/10000_vertices_to_protobuf - allocs/op |
70025 allocs/op |
70025 allocs/op |
1 |
BenchmarkTree/10000_vertices_from_protobuf |
151334739 ns/op 442172597 B/op 290039 allocs/op |
150604774 ns/op 442171429 B/op 290038 allocs/op |
1.00 |
BenchmarkTree/10000_vertices_from_protobuf - ns/op |
151334739 ns/op |
150604774 ns/op |
1.00 |
BenchmarkTree/10000_vertices_from_protobuf - B/op |
442172597 B/op |
442171429 B/op |
1.00 |
BenchmarkTree/10000_vertices_from_protobuf - allocs/op |
290039 allocs/op |
290038 allocs/op |
1.00 |
BenchmarkTree/20000_vertices_to_protobuf |
7453900 ns/op 12716977 B/op 140028 allocs/op |
7392561 ns/op 12716973 B/op 140028 allocs/op |
1.01 |
BenchmarkTree/20000_vertices_to_protobuf - ns/op |
7453900 ns/op |
7392561 ns/op |
1.01 |
BenchmarkTree/20000_vertices_to_protobuf - B/op |
12716977 B/op |
12716973 B/op |
1.00 |
BenchmarkTree/20000_vertices_to_protobuf - allocs/op |
140028 allocs/op |
140028 allocs/op |
1 |
BenchmarkTree/20000_vertices_from_protobuf |
681365404 ns/op 1697268120 B/op 580045 allocs/op |
681288692 ns/op 1697267944 B/op 580043 allocs/op |
1.00 |
BenchmarkTree/20000_vertices_from_protobuf - ns/op |
681365404 ns/op |
681288692 ns/op |
1.00 |
BenchmarkTree/20000_vertices_from_protobuf - B/op |
1697268120 B/op |
1697267944 B/op |
1.00 |
BenchmarkTree/20000_vertices_from_protobuf - allocs/op |
580045 allocs/op |
580043 allocs/op |
1.00 |
BenchmarkTree/30000_vertices_to_protobuf |
12138981 ns/op 19318503 B/op 210031 allocs/op |
11834310 ns/op 19318330 B/op 210030 allocs/op |
1.03 |
BenchmarkTree/30000_vertices_to_protobuf - ns/op |
12138981 ns/op |
11834310 ns/op |
1.03 |
BenchmarkTree/30000_vertices_to_protobuf - B/op |
19318503 B/op |
19318330 B/op |
1.00 |
BenchmarkTree/30000_vertices_to_protobuf - allocs/op |
210031 allocs/op |
210030 allocs/op |
1.00 |
BenchmarkTree/30000_vertices_from_protobuf |
1632779159 ns/op 3752036600 B/op 870053 allocs/op |
1623287411 ns/op 3752062344 B/op 870146 allocs/op |
1.01 |
BenchmarkTree/30000_vertices_from_protobuf - ns/op |
1632779159 ns/op |
1623287411 ns/op |
1.01 |
BenchmarkTree/30000_vertices_from_protobuf - B/op |
3752036600 B/op |
3752062344 B/op |
1.00 |
BenchmarkTree/30000_vertices_from_protobuf - allocs/op |
870053 allocs/op |
870146 allocs/op |
1.00 |
This comment was automatically generated by workflow using github-action-benchmark.
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- server/backend/database/testcases/testcases.go (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- server/backend/database/testcases/testcases.go
|
I've written test code to cover what was mentioned in the PR text. But I'm worried that the test case for the database has too business-like terms in it. Also, I'm not sure how to express in the test that the If you have any suggestions, please feel free to let me know. |
sejongk
reviewed
Aug 23, 2024
There was a problem hiding this comment.
Actionable comments posted: 1
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- server/backend/database/testcases/testcases.go (1 hunks)
Additional comments not posted (3)
server/backend/database/testcases/testcases.go (3)
346-351: LGTM!The function declaration and initial setup look good.
The code changes are approved.
382-458: Remove the ineffectual assignment toerr.The assignment to
erris not used and can be removed to clean up the code.Apply this diff to fix the issue:
snapshotInfo, _ := db.FindClosestSnapshotInfo( ctx, docRefKey, updatedDocInfo.ServerSeq, false, ) changeInfos, _ := db.FindChangeInfosBetweenServerSeqs( ctx, docRefKey, snapshotInfo.ServerSeq+1, updatedDocInfo.ServerSeq, ) + assert.NoError(t, err)Likely invalid or redundant comment.
352-380: Remove the ineffectual assignment toerr.The assignment to
erris not used and can be removed to clean up the code.Apply this diff to fix the issue:
- updatedClientInfo, _ := db.FindClientInfoByRefKey(ctx, clientInfo.RefKey()) + updatedClientInfo, err := db.FindClientInfoByRefKey(ctx, clientInfo.RefKey()) + assert.NoError(t, err)Likely invalid or redundant comment.
|
@hackerwins Would you mind reviewing all the changes? |
hackerwins
reviewed
Sep 2, 2024
There was a problem hiding this comment.
It looks good overall. I am curious about the following:
This change minimizes unnecessary database resource consumption, particularly in scenarios where the most recent document editor, User A, continues editing without any interference from other users (B, C, etc.).
Could you explain me how to reproduce this scenario? I want to ensure there isn't another underlying issue.
hackerwins
approved these changes
Sep 2, 2024
Reproducing at the user level is very simple: Here is an example scenario: A. Open the CodeMirror example of the JS SDK in a single browser In the above scenario, unnecessary queries were still occurring. |
2 tasks
raararaara
pushed a commit
that referenced
this pull request
Oct 7, 2024
#974) This commit modifies the FindChangeInfosBetweenServerSeqs method to avoid executing DB queries when the from > to. This change minimizes unnecessary database resource consumption, particularly in scenarios where the most recent document editor, User A, continues editing without any interference from other users (B, C, etc.). In such cases, in PushPull, User A's Checkpoint.serverSeq always equal to the server's initialServerSeq (DocInfo.serverSeq), leading to a situation where from > to in the FindChangeInfosBetweenServerSeqs. By preventing the execution of a query under these circumstances, we can reduce the load on database resources.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What this PR does / why we need it:
This PR modifies the
FindChangeInfosBetweenServerSeqsmethod to avoid executing DB queries when thefrom>to. This change minimizes unnecessary database resource consumption, particularly in scenarios where the most recent document editor, User A, continues editing without any interference from other users (B, C, etc.).In such cases, in PushPull, User A's
Checkpoint.serverSeqalways equal to the server'sinitialServerSeq(DocInfo.serverSeq), leading to a situation wherefrom > toin theFindChangeInfosBetweenServerSeqsmethod. By preventing the execution of a query under these circumstances, we can reduce the load on database resources.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
Summary by CodeRabbit
Bug Fixes
Tests