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

Paste performance for large images #10287

Closed
FlowIT-JIT opened this issue Aug 2, 2021 · 1 comment · Fixed by #10367
Closed

Paste performance for large images #10287

FlowIT-JIT opened this issue Aug 2, 2021 · 1 comment · Fixed by #10367
Assignees
Labels
intro Good first ticket. squad:core Issue to be handled by the Core team. type:bug This issue reports a buggy (incorrect) behavior.

Comments

@FlowIT-JIT
Copy link

FlowIT-JIT commented Aug 2, 2021

📝 Provide detailed reproduction steps (if any)

You can increase paste performance in Chrome with a very simple optimization.
Due to missing caching in DataTransfer's implementation in Chrome (see bug report here), accessing the files propery is expensive.
On my fairly fast MacBook Pro 2017 (2,8 GHz Intel Core i7, 16 Gigs of memory) it takes abount 1 second to read the files collection of the DataTransfer object if I paste an image with a size of approx. 17 MB. So the paste operation takes about 2 seconds in CKEditor 5.

image

If you avoid accessing the files collection more than once, you can halve the time.

image

The test was performed on the official online demo:
https://ckeditor.com/ckeditor-5/demo/

A similar bug report has been filed for CKEditor 4:
ckeditor/ckeditor4#4807
-- Although the problem is much more noticable in v4 since the files collection is being accessed multiple times.

In CKEditor 5 this is a minor problem since the collection is only accessed twice. But it's also easy to fix to give us a noticable performance improvement for large images.

✔️ Expected result

What is the expected result of the above steps?

❌ Actual result

What is the actual result of the above steps?

❓ Possible solution

If you have ideas, you can list them here. Otherwise, you can delete this section.

📃 Other details

  • Browser: Chrome Version 92.0.4515.107 (Official Build) (x86_64)
  • OS: macOS 10.13.6 (17G14033)
  • First affected CKEditor version: …
  • Installed CKEditor plugins: …

If you'd like to see this fixed sooner, add a 👍 reaction to this post.

@FlowIT-JIT FlowIT-JIT added the type:bug This issue reports a buggy (incorrect) behavior. label Aug 2, 2021
@FlowIT-JIT FlowIT-JIT changed the title Paste performance Paste performance for large images Aug 2, 2021
@Reinmar Reinmar added the intro Good first ticket. label Aug 2, 2021
@Reinmar
Copy link
Member

Reinmar commented Aug 2, 2021

Thanks for the tip ❤️

@Reinmar Reinmar added the squad:core Issue to be handled by the Core team. label Aug 2, 2021
@Reinmar Reinmar added this to the nice-to-have milestone Aug 2, 2021
@Reinmar Reinmar modified the milestones: nice-to-have, iteration 46 Aug 10, 2021
niegowski added a commit that referenced this issue Aug 17, 2021
Other (clipboard): The `DataTransfer.files` property is not evaluated more than once. Closes #10287.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
intro Good first ticket. squad:core Issue to be handled by the Core team. type:bug This issue reports a buggy (incorrect) behavior.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants