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

fix: Improve edit processing #2247

Merged
merged 7 commits into from
Jun 1, 2023
Merged

fix: Improve edit processing #2247

merged 7 commits into from
Jun 1, 2023

Conversation

dordsor21
Copy link
Member

Create a copy of SET chunk before processing

  • Completely nullifies issue of writing to SET between processing submission and processing completion
  • Leads to larger memory footprint, but that is acceptable
  • Leads to performance improvement in processing as there is no need to perform any multi-threading protection operations
  • Fixes Undo of big pastes does not remove all blocks #1861
  • Fixes Error | Chunk Queue Skipping #2074 (gonna go ahead and say it does, this is really the only thing I can think of to do with the issue, and it's super rare anyway (never replicated by anyone else))## Overview
  • *tack on some changes (increased default/recommended target size)
  • *tack on some changes (minor cleanup)
  • *tack on some changes (improve a couple of CharSetBlocks methods)

dordsor21 added 4 commits May 25, 2023 22:53
 - Completely nullifies issue of writing to SET between processing submission and processing completion
 - Leads to larger memory footprint, but that is acceptable
 - Leads to performance improvement in processing as there is no need to perform any multi-threading protection operations
 - Fixes #1861
 - Fixes #2074 (gonna go ahead and say it does, this is really the only thing I can think of to do with the issue, and it's super rare anyway (never replicated by anyone else))
@dordsor21 dordsor21 requested a review from a team as a code owner May 25, 2023 21:59
@github-actions github-actions bot added the Bugfix This PR fixes a bug label May 25, 2023
@SirYwell
Copy link
Member

This has some conflicts with #2181

@MattBDev
Copy link
Contributor

MattBDev commented May 26, 2023

Leads to larger memory footprint, but that is acceptable

I know our memory usage is already pretty good and can take a hit but I am curious as to how much more memory this uses. Did you have an estimate?

@dordsor21
Copy link
Member Author

dordsor21 commented May 26, 2023

Leads to larger memory footprint, but that is acceptable.

I know our memory usage is already pretty good and can take a hit but I am curious as to how much more memory this uses. Did you have an estimate?

0.5 to maybe 4 Mbit a chunk I guess (so 0.1 to 0.5 MB)

Copy link
Member

@SirYwell SirYwell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

found a typo but looks good otherwise

@dordsor21 dordsor21 requested a review from SirYwell May 31, 2023 15:52
@dordsor21 dordsor21 force-pushed the fix/copy-set-before-process branch from 4a0c7b2 to a5193ed Compare May 31, 2023 15:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bugfix This PR fixes a bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Error | Chunk Queue Skipping Undo of big pastes does not remove all blocks
3 participants