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

Do not store non-document operation in batches #17678

Open
scofalik opened this issue Dec 19, 2024 · 0 comments · May be fixed by #17679
Open

Do not store non-document operation in batches #17678

scofalik opened this issue Dec 19, 2024 · 0 comments · May be fixed by #17679
Assignees
Labels
domain:performance This issue reports a problem or potential improvement regarding editor performance. squad:collaboration Issue to be handled by the Collaboration team. type:improvement This issue reports a possible enhancement of an existing feature. type:performance This issue reports a performance issue or a possible performance improvement.

Comments

@scofalik
Copy link
Contributor

📝 Provide a description of the improvement

Non-document operations are operations that are changing something outside of the main model tree, e.g. inside detached model document fragments.

We use model.Writer for all kinds of operations on model items, also for creating model nodes before they are actually added to the model tree. Such operations have baseVersion set to null and isDocumentOperation set to false.

We want non-document operations to be transparent. We need them only for their execute() method, so that we apply the desired change. We filter non-document operations in most (all?) the logic that cares about the operations, because including them usually leads to bugs or unwanted behavior. For example, we filter them from undo mechanism, we don't send them in real-time collaboration as they are not needed on remote client etc.

In short, these operations are useless after they are applied. But they are added to Batches, like regular operations.

And the problem is that batches retain them. And this is a memory performance problem for big documents. Why exactly? First, we create massive amount of operations during upcast to create a model fragment from the initial data. This model fragment is then inserted into real model tree. But all the non-document operations used to create the model fragment are retained in the batch.

As measured, for an editor instance which takes 100MB memory, this optimization can save around 20MBs (tested for mixed test from our performance tests cases suite).

On a side note: I believe that for non-document circumstances, we might as well not use operations in the writer at all. It would be possible if "operation action" was declared as a static method of the operation. So we'd call InsertOperation.execute( ... ) in non-document context, instead of const operation = new InsertOperation( ... ); operation.execute();. This is a possible next step improvement that may have a positive impact on the editor performance.

@scofalik scofalik added domain:performance This issue reports a problem or potential improvement regarding editor performance. squad:collaboration Issue to be handled by the Collaboration team. type:improvement This issue reports a possible enhancement of an existing feature. type:performance This issue reports a performance issue or a possible performance improvement. labels Dec 19, 2024
@scofalik scofalik self-assigned this Dec 19, 2024
@scofalik scofalik linked a pull request Dec 19, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
domain:performance This issue reports a problem or potential improvement regarding editor performance. squad:collaboration Issue to be handled by the Collaboration team. type:improvement This issue reports a possible enhancement of an existing feature. type:performance This issue reports a performance issue or a possible performance improvement.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant