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

snapshotToDoc very slow with large snapshot #8380

Closed
kerituni12 opened this issue Sep 17, 2024 · 4 comments · Fixed by #8428
Closed

snapshotToDoc very slow with large snapshot #8380

kerituni12 opened this issue Sep 17, 2024 · 4 comments · Fixed by #8428
Labels
mod:clipboard Module: related to clipboard type:perf Performance related affairs

Comments

@kerituni12
Copy link

The performance is slower due to the use of nextTick in the _snapshotToBlock method. Specifically, I see the following code:

await new Promise(resolve => nextTick(() => resolve(undefined)));

While nextTick is useful for handling large content during pasting, it can slow down operations when used in snapshotToDoc. It would be beneficial to add a parameter like usingNextTick to snapshotToDoc or explore other approaches to mitigate this issue.

@doodlewind doodlewind added type:perf Performance related affairs mod:clipboard Module: related to clipboard labels Sep 18, 2024
@R151831
Copy link

R151831 commented Sep 20, 2024

@doodlewind

We are encountering the same issue. Can we proceed with removing the nextTick line?
Please let us know if there are any potential consequences of doing so.

@RajaTheKing826
Copy link

I'm also facing this issue from recent days. Don't we have any fix for this.

@EddiePengg
Copy link

我也遇到了这个问题,希望在json的处理上能够更优秀,这样就有更通用的语言,可以被更多项目使用

doodlewind added a commit that referenced this issue Oct 14, 2024
Close #8380

## Problem

The original process of converting snapshot JSON to blocks was significantly slowed to ensure UI responsiveness. It involved two types of async computations:

* Asynchronously converting snapshots to draft models.
* Asynchronously inserting draft models into the block tree using `doc.addBlock`.

Original import sequence:

```
Convert snapshot node A
Insert block A
Wait for tick

Convert snapshot node B
Insert block B
Wait for tick

Convert snapshot node C
Insert block C
Wait for tick
...
```

Despite `doc.addBlock` being synchronous, a `requestAnimationFrame` interval was added after each block insertion to prevent UI blocking. This resulted in long processing times when pasting thousands of blocks:

https://github.com/user-attachments/assets/3a6379c9-621e-490e-a79f-4377494d5915

## New Design

This PR reimplements the process:

1. Convert all snapshot nodes first, either using `Promise.all` or sequential awaits: Firstly we flatten the block tree, asynchronously convert all nodes, then rebuild the tree.
2. Insert the converted draft block tree in batches, **waiting for a tick after each _batch_ instead of after each _block_**.

This optimizes both block tree conversion and insertion while maintaining UI responsiveness.

## Performance Tests

### Comparing serial vs parallel (`Promise.all`) snapshot tree conversion:

- 1000 blocks: `Promise.all` 4.8ms, series 2.8ms
- 60 images: `Promise.all` 2.8ms, series 2.2ms

Serial conversion proved faster and was implemented in b90d736. The flatten-rebuild logic was retained for scalability.

### Pasting 1000 blocks performance:

Original: ~10s (see screen recording above)

New: ~1s with maintained UI responsiveness

https://github.com/user-attachments/assets/0202330a-66ce-47f3-b220-44a4e9c7c37d

### Switching to imported documents:

Loading the block tree into DOM (by switching to an imported document) causes similar delays in both implementations.

---

Legacy first take:

<details>

This PR significantly improves the performance of block import operations by addressing several inefficiencies in the current implementation:

- Removed the need to wait for `requestAnimationFrame` after each block insertion (see #7796), which was causing significant slowdowns.
- <del>Fixed the batch mechanism (see #7790) that wasn't properly enabled, replacing serial execution with true parallel processing.</del>
- Instead of executing all remaining tasks at once after 100 items (still causing UI freezes), we now process tasks in consistent batches of 100 per tick.
- Replaced `setTimeout` with `requestIdleCallback` for batch ticks, better utilizing idle browser time.

These optimizations result in much faster block imports while maintaining UI responsiveness. The new implementation processes 100 tasks per tick, striking a balance between speed and smooth user experience.

Tested using [clipboard-test.md](https://github.com/user-attachments/files/17088183/clipboard-test.md) with 1000 line and 600 blocks:

## Before (took ~10s to paste)

https://github.com/user-attachments/assets/3a6379c9-621e-490e-a79f-4377494d5915

## After (instant response without blocking UI)

https://github.com/user-attachments/assets/dea7c1f9-a8c1-4926-92b7-f5cb40957fad

</details>
@doodlewind
Copy link
Member

Closed by #8428

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
mod:clipboard Module: related to clipboard type:perf Performance related affairs
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

5 participants