Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Slow apply when garbage collecting large update #541

Open
2 tasks done
NilSet opened this issue Jun 16, 2023 — with Huly GitHub · 6 comments
Open
2 tasks done

Slow apply when garbage collecting large update #541

NilSet opened this issue Jun 16, 2023 — with Huly GitHub · 6 comments
Assignees
Labels

Comments

Copy link
Contributor

NilSet commented Jun 16, 2023

Checklist

Describe the bug Given a large, ungarbagecollected doc which was persisted as an encoded update, if you apply the update to a document with garbage collection enabled, the apply can take a very long time. Based on my profiling, the time is all in tryToMergeWithLeft, possibly due to calling structs.splice(pos, 1) many times, requiring the shifting of a very large array each time.

To Reproduce A anonymized document which reproduces this problem can be found here: https://github.com/NilSet/yjs-stack-overflow/tree/slow-gc On my machine it takes 38 seconds to apply the document with garbage collection enabled. Note that if you change the test script to disable gc on the doc, it only takes 1.6 seconds to apply the document.

Expected behavior A clear and concise description of what you expected to happen.

Screenshots If applicable, add screenshots to help explain your problem.

Environment Information

  • Node 18
  • Yjs 13.6.4

Huly®: YJS-373

@lijie1129
Copy link

lijie1129 commented Jun 29, 2023

hi, @NilSet

This issue is not reproduce on the Yjs 13.6.5 via your repo .

2023-06-29.18.46.18.mov

Checklist

Describe the bug Given a large, ungarbagecollected doc which was persisted as an encoded update, if you apply the update to a document with garbage collection enabled, the apply can take a very long time. Based on my profiling, the time is all in tryToMergeWithLeft, possibly due to calling structs.splice(pos, 1) many times, requiring the shifting of a very large array each time.

To Reproduce A anonymized document which reproduces this problem can be found here: https://github.com/NilSet/yjs-stack-overflow/tree/slow-gc On my machine it takes 38 seconds to apply the document with garbage collection enabled. Note that if you change the test script to disable gc on the doc, it only takes 1.6 seconds to apply the document.

Expected behavior A clear and concise description of what you expected to happen.

Screenshots If applicable, add screenshots to help explain your problem.

Environment Information

  • Node 18
  • Yjs 13.6.4

@NilSet
Copy link
Contributor Author

NilSet commented Jun 29, 2023

You are on main branch, this issue links to the slow-gc branch which checks a different anonymized sample with different characteristics than the main branch.

@lijie1129
Copy link

You are on main branch, this issue links to the slow-gc branch which checks a different anonymized sample with different characteristics than the main branch.

Hi, @NilSet

I swithed the slow-iterate-deleted-structs branch and reproduce it. 🤝

And this issue is not reproduce on the Yjs 14.0.0-1

2023-06-30.10.33.42.mov

@NilSet
Copy link
Contributor Author

NilSet commented Jul 4, 2023

You are still on the wrong branch.

@dmonad dmonad closed this as completed in c77dedb Jul 17, 2023
dmonad added a commit that referenced this issue Jul 17, 2023
@dmonad dmonad reopened this Jul 17, 2023
@dmonad
Copy link
Member

dmonad commented Jul 17, 2023

I published a fix in 13.6.7. I will close this ticket when I/somebody confirms this is fixed.

@dmonad
Copy link
Member

dmonad commented Jul 17, 2023

@NilSet I can run your reproduction repo now in 9 seconds (45 seconds using Yjs@v13.6.4).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants