Fix slow loading/editing of sheet with ~800 comments [master] #3556
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
Profiling points to updateReplyCount() as the bottleneck where it does O(N^3) work where N is the number of comments. The innermost loop is used to find the index of a comment-section in an array using its 'id'. Turns out that flattening this loop to a < log(N) lookup via a js map from id -> index pretty much cures the perceived 'slowness'. But we need to ensure the correctness of the map by updating it whenever 'commentList' is mutated. Time to load the document with 800 comments (arranged in 50 rows x 16 cols table): Before the patch: 1min 31 sec After the patch: 3.5 sec After-load performance (comment popup/hide/scroll) with this patch is similar to how it feels in co-6-4 which does not have this issue. Signed-off-by: Dennis Francis <dennis.francis@collabora.com> Change-Id: I4c47f14767cac4c7e9744344d24f646dd27302a6
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.
Profiling points to updateReplyCount() as the bottleneck where it does
O(N^3) work where N is the number of comments. The innermost loop is
used to find the index of a comment-section in an array using its 'id'.
Turns out that flattening this loop to a < log(N) lookup via a js map
from id -> index pretty much cures the perceived 'slowness'. But we need
to ensure the correctness of the map by updating it whenever
'commentList' is mutated.
Time to load the document with 800 comments (arranged in 50 rows x 16
cols table):
Before the patch: 1min 31 sec
After the patch: 3.5 sec
After-load performance (comment popup/hide/scroll) with this patch is
similar to how it feels in co-6-4 which does not have this issue.
Signed-off-by: Dennis Francis dennis.francis@collabora.com
Change-Id: I4c47f14767cac4c7e9744344d24f646dd27302a6
Checklist
make checkmake runand manually verified that everything looks okay