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

getData can be very slow depending on content #14245

Closed
agradl opened this issue May 24, 2023 · 5 comments
Closed

getData can be very slow depending on content #14245

agradl opened this issue May 24, 2023 · 5 comments
Labels
domain:performance This issue reports a problem or potential improvement regarding editor performance. package:watchdog resolution:expired This issue was closed due to lack of feedback. type:bug This issue reports a buggy (incorrect) behavior.

Comments

@agradl
Copy link

agradl commented May 24, 2023

getData can be very slow depending on content

📝 Provide detailed reproduction steps (if any)

run the following code in the demo

var htmlData = "<p>Ho<strong>w do</strong> we <span style=\"font-family:'Courier New', Courier, monospace;\">get the<s> cont</s>ra</span>c<span style=\"font-size:18px;\"><strong>ts transferr</strong></span>ed over<span style=\"background-color:hsl(0,75%,60%);\"> from Tens</span>or to CQP<br>Ho<strong>w do</strong> we <span style=\"font-family:'Courier New', Courier, monospace;\">get the<s> cont</s>ra</span>c<span style=\"font-size:18px;\"><strong>ts transferr</strong></span>ed over<span style=\"background-color:hsl(0,75%,60%);\"> from Tens</span>or to CQP<br>Ho<strong>w do</strong> we <span style=\"font-family:'Courier New', Courier, monospace;\">get the<s> cont</s>ra</span>c<span style=\"font-size:18px;\"><strong>ts transferr</strong></span>ed over<span style=\"background-color:hsl(0,75%,60%);\"> from Tens</span>or to CQP<br>Ho<strong>w do</strong> we <span style=\"font-family:'Courier New', Courier, monospace;\">get the<s> cont</s>ra</span>c<span style=\"font-size:18px;\"><strong>ts transferr</strong></span>ed over<span style=\"background-color:hsl(0,75%,60%);\"> from Tens</span>or to CQP<br>Ho<strong>w do</strong> we <span style=\"font-family:'Courier New', Courier, monospace;\">get the<s> cont</s>ra</span>c<span style=\"font-size:18px;\"><strong>ts transferr</strong></span>ed over<span style=\"background-color:hsl(0,75%,60%);\"> from Tens</span>or to CQP<br>Ho<strong>w do</strong> we <span style=\"font-family:'Courier New', Courier, monospace;\">get the<s> cont</s>ra</span>c<span style=\"font-size:18px;\"><strong>ts transferr</strong></span>ed over<span style=\"background-color:hsl(0,75%,60%);\"> from Tens</span>or to CQP<br>Ho<strong>w do</strong> we <span style=\"font-family:'Courier New', Courier, monospace;\">get the<s> cont</s>ra</span>c<span style=\"font-size:18px;\"><strong>ts transferr</strong></span>ed over<span style=\"background-color:hsl(0,75%,60%);\"> from Tens</span>or to CQP<br>Ho<strong>w do</strong> we <span style=\"font-family:'Courier New', Courier, monospace;\">get the<s> cont</s>ra</span>c<span style=\"font-size:18px;\"><strong>ts transferr</strong></span>ed over<span style=\"background-color:hsl(0,75%,60%);\"> from Tens</span>or to CQP<br>Ho<strong>w do</strong> we <span style=\"font-family:'Courier New', Courier, monospace;\">get the<s> cont</s>ra</span>c<span style=\"font-size:18px;\"><strong>ts transferr</strong></span>ed over<span style=\"background-color:hsl(0,75%,60%);\"> from Tens</span>or to CQP<br>Ho<strong>w do</strong> we <span style=\"font-family:'Courier New', Courier, monospace;\">get the<s> cont</s>ra</span>c<span style=\"font-size:18px;\"><strong>ts transferr</strong></span>ed over<span style=\"background-color:hsl(0,75%,60%);\"> from Tens</span>or to CQP<br>Ho<strong>w do</strong> we <span style=\"font-family:'Courier New', Courier, monospace;\">get the<s> cont</s>ra</span>c<span style=\"font-size:18px;\"><strong>ts transferr</strong></span>ed over<span style=\"background-color:hsl(0,75%,60%);\"> from Tens</span>or to CQP<br>Ho<strong>w do</strong> we <span style=\"font-family:'Courier New', Courier, monospace;\">get the<s> cont</s>ra</span>c<span style=\"font-size:18px;\"><strong>ts transferr</strong></span>ed over<span style=\"background-color:hsl(0,75%,60%);\"> from Tens</span>or to CQP<br>Ho<strong>w do</strong> we <span style=\"font-family:'Courier New', Courier, monospace;\">get the<s> cont</s>ra</span>c<span style=\"font-size:18px;\"><strong>ts transferr</strong></span>ed over<span style=\"background-color:hsl(0,75%,60%);\"> from Tens</span>or to CQP<br>Ho<strong>w do</strong> we <span style=\"font-family:'Courier New', Courier, monospace;\">get the<s> cont</s>ra</span>c<span style=\"font-size:18px;\"><strong>ts transferr</strong></span>ed over<span style=\"background-color:hsl(0,75%,60%);\"> from Tens</span>or to CQP<br>Ho<strong>w do</strong> we <span style=\"font-family:'Courier New', Courier, monospace;\">get the<s> cont</s>ra</span>c<span style=\"font-size:18px;\"><strong>ts transferr</strong></span>ed over<span style=\"background-color:hsl(0,75%,60%);\"> from Tens</span>or to CQP<br>Ho<strong>w do</strong> we <span style=\"font-family:'Courier New', Courier, monospace;\">get the<s> cont</s>ra</span>c<span style=\"font-size:18px;\"><strong>ts transferr</strong></span>ed over<span style=\"background-color:hsl(0,75%,60%);\"> from Tens</span>or to CQP<br>Ho<strong>w do</strong> we <span style=\"font-family:'Courier New', Courier, monospace;\">get the<s> cont</s>ra</span>c<span style=\"font-size:18px;\"><strong>ts transferr</strong></span>ed over<span style=\"background-color:hsl(0,75%,60%);\"> from Tens</span>or to CQP<br>Ho<strong>w do</strong> we <span style=\"font-family:'Courier New', Courier, monospace;\">get the<s> cont</s>ra</span>c<span style=\"font-size:18px;\"><strong>ts transferr</strong></span>ed over<span style=\"background-color:hsl(0,75%,60%);\"> from Tens</span>or to CQP<br>Ho<strong>w do</strong> we <span style=\"font-family:'Courier New', Courier, monospace;\">get the<s> cont</s>ra</span>c<span style=\"font-size:18px;\"><strong>ts transferr</strong></span>ed over<span style=\"background-color:hsl(0,75%,60%);\"> from Tens</span>or to CQP<br>Ho<strong>w do</strong> we <span style=\"font-family:'Courier New', Courier, monospace;\">get the<s> cont</s>ra</span>c<span style=\"font-size:18px;\"><strong>ts transferr</strong></span>ed over<span style=\"background-color:hsl(0,75%,60%);\"> from Tens</span>or to CQP<br>Ho<strong>w do</strong> we <span style=\"font-family:'Courier New', Courier, monospace;\">get the<s> cont</s>ra</span>c<span style=\"font-size:18px;\"><strong>ts transferr</strong></span>ed over<span style=\"background-color:hsl(0,75%,60%);\"> from Tens</span>or to CQP<br>Ho<strong>w do</strong> we <span style=\"font-family:'Courier New', Courier, monospace;\">get the<s> cont</s>ra</span>c<span style=\"font-size:18px;\"><strong>ts transferr</strong></span>ed over<span style=\"background-color:hsl(0,75%,60%);\"> from Tens</span>or to CQP<br>Ho<strong>w do</strong> we <span style=\"font-family:'Courier New', Courier, monospace;\">get the<s> cont</s>ra</span>c<span style=\"font-size:18px;\"><strong>ts transferr</strong></span>ed over<span style=\"background-color:hsl(0,75%,60%);\"> from Tens</span>or to CQP<br>Ho<strong>w do</strong> we <span style=\"font-family:'Courier New', Courier, monospace;\">get the<s> cont</s>ra</span>c<span style=\"font-size:18px;\"><strong>ts transferr</strong></span>ed over<span style=\"background-color:hsl(0,75%,60%);\"> from Tens</span>or to CQP<br>Ho<strong>w do</strong> we <span style=\"font-family:'Courier New', Courier, monospace;\">get the<s> cont</s>ra</span>c<span style=\"font-size:18px;\"><strong>ts transferr</strong></span>ed over<span style=\"background-color:hsl(0,75%,60%);\"> from Tens</span>or to CQP<br>Ho<strong>w do</strong> we <span style=\"font-family:'Courier New', Courier, monospace;\">get the<s> cont</s>ra</span>c<span style=\"font-size:18px;\"><strong>ts transferr</strong></span>ed over<span style=\"background-color:hsl(0,75%,60%);\"> from Tens</span>or to CQP<br>Ho<strong>w do</strong> we <span style=\"font-family:'Courier New', Courier, monospace;\">get the<s> cont</s>ra</span>c<span style=\"font-size:18px;\"><strong>ts transferr</strong></span>ed over<span style=\"background-color:hsl(0,75%,60%);\"> from Tens</span>or to CQP<br>Ho<strong>w do</strong> we <span style=\"font-family:'Courier New', Courier, monospace;\">get the<s> cont</s>ra</span>c<span style=\"font-size:18px;\"><strong>ts transferr</strong></span>ed over<span style=\"background-color:hsl(0,75%,60%);\"> from Tens</span>or to CQP</p><p>Ho<strong>w do</strong> we <span style=\"font-family:'Courier New', Courier, monospace;\">get the<s> cont</s>ra</span>c<span style=\"font-size:18px;\"><strong>ts transferr</strong></span>ed over<span style=\"background-color:hsl(0,75%,60%);\"> from Tens</span>or to CQP<br>Ho<strong>w do</strong> we <span style=\"font-family:'Courier New', Courier, monospace;\">get the<s> cont</s>ra</span>c<span style=\"font-size:18px;\"><strong>ts transferr</strong></span>ed over<span style=\"background-color:hsl(0,75%,60%);\"> from Tens</span>or to CQP<br>Ho<strong>w do</strong> we <span style=\"font-family:'Courier New', Courier, monospace;\">get the<s> cont</s>ra</span>c<span style=\"font-size:18px;\"><strong>ts transferr</strong></span>ed over<span style=\"background-color:hsl(0,75%,60%);\"> from Tens</span>or to CQP<br>Ho<strong>w do</strong> we <span style=\"font-family:'Courier New', Courier, monospace;\">get the<s> cont</s>ra</span>c<span style=\"font-size:18px;\"><strong>ts transferr</strong></span>ed over<span style=\"background-color:hsl(0,75%,60%);\"> from Tens</span>or to CQP<br>Ho<strong>w do</strong> we <span style=\"font-family:'Courier New', Courier, monospace;\">get the<s> cont</s>ra</span>c<span style=\"font-size:18px;\"><strong>ts transferr</strong></span>ed over<span style=\"background-color:hsl(0,75%,60%);\"> from Tens</span>or to CQP<br>Ho<strong>w do</strong> we <span style=\"font-family:'Courier New', Courier, monospace;\">get the<s> cont</s>ra</span>c<span style=\"font-size:18px;\"><strong>ts transferr</strong></span>ed over<span style=\"background-color:hsl(0,75%,60%);\"> from Tens</span>or to CQP<br>Ho<strong>w do</strong> we <span style=\"font-family:'Courier New', Courier, monospace;\">get the<s> cont</s>ra</span>c<span style=\"font-size:18px;\"><strong>ts transferr</strong></span>ed over<span style=\"background-color:hsl(0,75%,60%);\"> from Tens</span>or to CQP<br>Ho<strong>w do</strong> we <span style=\"font-family:'Courier New', Courier, monospace;\">get the<s> cont</s>ra</span>c<span style=\"font-size:18px;\"><strong>ts transferr</strong></span>ed over<span style=\"background-color:hsl(0,75%,60%);\"> from Tens</span>or to CQP<br>Ho<strong>w do</strong> we <span style=\"font-family:'Courier New', Courier, monospace;\">get the<s> cont</s>ra</span>c<span style=\"font-size:18px;\"><strong>ts transferr</strong></span>ed over<span style=\"background-color:hsl(0,75%,60%);\"> from Tens</span>or to CQP<br>Ho<strong>w do</strong> we <span style=\"font-family:'Courier New', Courier, monospace;\">get the<s> cont</s>ra</span>c<span style=\"font-size:18px;\"><strong>ts transferr</strong></span>ed over<span style=\"background-color:hsl(0,75%,60%);\"> from Tens</span>or to CQP<br>Ho<strong>w do</strong> we <span style=\"font-family:'Courier New', Courier, monospace;\">get the<s> cont</s>ra</span>c<span style=\"font-size:18px;\"><strong>ts transferr</strong></span>ed over<span style=\"background-color:hsl(0,75%,60%);\"> from Tens</span>or to CQP<br>Ho<strong>w do</strong> we <span style=\"font-family:'Courier New', Courier, monospace;\">get the<s> cont</s>ra</span>c<span style=\"font-size:18px;\"><strong>ts transferr</strong></span>ed over<span style=\"background-color:hsl(0,75%,60%);\"> from Tens</span>or to CQP<br>Ho<strong>w do</strong> we <span style=\"font-family:'Courier New', Courier, monospace;\">get the<s> cont</s>ra</span>c<span style=\"font-size:18px;\"><strong>ts transferr</strong></span>ed over<span style=\"background-color:hsl(0,75%,60%);\"> from Tens</span>or to CQP<br>Ho<strong>w do</strong> we <span style=\"font-family:'Courier New', Courier, monospace;\">get the<s> cont</s>ra</span>c<span style=\"font-size:18px;\"><strong>ts transferr</strong></span>ed over<span style=\"background-color:hsl(0,75%,60%);\"> from Tens</span>or to CQP<br>Ho<strong>w do</strong> we <span style=\"font-family:'Courier New', Courier, monospace;\">get the<s> cont</s>ra</span>c<span style=\"font-size:18px;\"><strong>ts transferr</strong></span>ed over<span style=\"background-color:hsl(0,75%,60%);\"> from Tens</span>or to CQP<br>Ho<strong>w do</strong> we <span style=\"font-family:'Courier New', Courier, monospace;\">get the<s> cont</s>ra</span>c<span style=\"font-size:18px;\"><strong>ts transferr</strong></span>ed over<span style=\"background-color:hsl(0,75%,60%);\"> from Tens</span>or to CQP<br>Ho<strong>w do</strong> we <span style=\"font-family:'Courier New', Courier, monospace;\">get the<s> cont</s>ra</span>c<span style=\"font-size:18px;\"><strong>ts transferr</strong></span>ed over<span style=\"background-color:hsl(0,75%,60%);\"> from Tens</span>or to CQP<br>Ho<strong>w do</strong> we <span style=\"font-family:'Courier New', Courier, monospace;\">get the<s> cont</s>ra</span>c<span style=\"font-size:18px;\"><strong>ts transferr</strong></span>ed over<span style=\"background-color:hsl(0,75%,60%);\"> from Tens</span>or to CQP<br>Ho<strong>w do</strong> we <span style=\"font-family:'Courier New', Courier, monospace;\">get the<s> cont</s>ra</span>c<span style=\"font-size:18px;\"><strong>ts transferr</strong></span>ed over<span style=\"background-color:hsl(0,75%,60%);\"> from Tens</span>or to CQP<br>Ho<strong>w do</strong> we <span style=\"font-family:'Courier New', Courier, monospace;\">get the<s> cont</s>ra</span>c<span style=\"font-size:18px;\"><strong>ts transferr</strong></span>ed over<span style=\"background-color:hsl(0,75%,60%);\"> from Tens</span>or to CQP<br>Ho<strong>w do</strong> we <span style=\"font-family:'Courier New', Courier, monospace;\">get the<s> cont</s>ra</span>c<span style=\"font-size:18px;\"><strong>ts transferr</strong></span>ed over<span style=\"background-color:hsl(0,75%,60%);\"> from Tens</span>or to CQP<br>Ho<strong>w do</strong> we <span style=\"font-family:'Courier New', Courier, monospace;\">get the<s> cont</s>ra</span>c<span style=\"font-size:18px;\"><strong>ts transferr</strong></span>ed over<span style=\"background-color:hsl(0,75%,60%);\"> from Tens</span>or to CQP<br>Ho<strong>w do</strong> we <span style=\"font-family:'Courier New', Courier, monospace;\">get the<s> cont</s>ra</span>c<span style=\"font-size:18px;\"><strong>ts transferr</strong></span>ed over<span style=\"background-color:hsl(0,75%,60%);\"> from Tens</span>or to CQP<br>Ho<strong>w do</strong> we <span style=\"font-family:'Courier New', Courier, monospace;\">get the<s> cont</s>ra</span>c<span style=\"font-size:18px;\"><strong>ts transferr</strong></span>ed over<span style=\"background-color:hsl(0,75%,60%);\"> from Tens</span>or to CQP<br>Ho<strong>w do</strong> we <span style=\"font-family:'Courier New', Courier, monospace;\">get the<s> cont</s>ra</span>c<span style=\"font-size:18px;\"><strong>ts transferr</strong></span>ed over<span style=\"background-color:hsl(0,75%,60%);\"> from Tens</span>or to CQP<br>Ho<strong>w do</strong> we <span style=\"font-family:'Courier New', Courier, monospace;\">get the<s> cont</s>ra</span>c<span style=\"font-size:18px;\"><strong>ts transferr</strong></span>ed over<span style=\"background-color:hsl(0,75%,60%);\"> from Tens</span>or to CQP<br>Ho<strong>w do</strong> we <span style=\"font-family:'Courier New', Courier, monospace;\">get the<s> cont</s>ra</span>c<span style=\"font-size:18px;\"><strong>ts transferr</strong></span>ed over<span style=\"background-color:hsl(0,75%,60%);\"> from Tens</span>or to CQP<br>Ho<strong>w do</strong> we <span style=\"font-family:'Courier New', Courier, monospace;\">get the<s> cont</s>ra</span>c<span style=\"font-size:18px;\"><strong>ts transferr</strong></span>ed over<span style=\"background-color:hsl(0,75%,60%);\"> from Tens</span>or to CQP</p>";
var ckEditorInstance = document.querySelector('.ck-content.ck-editor__editable').ckeditorInstance;
ckEditorInstance.setData(htmlData);
var sampleSize = 5;
var time = 0;
for (var i = 0; i < sampleSize; i++) {
	var start = new Date().getTime();
	ckEditorInstance.getData();
    var elapsed = (new Date().getTime() - start);
    console.log('sample ' + i + ' took ' + elapsed + 'ms');
    time += elapsed;
}
console.log('averaged ' + time/sampleSize + 'ms with a sample size of ' + sampleSize);

✔️ Expected result

getData returns in reasonable time and is possibly even cached until the model is changed so subsequent calls return in less time

❌ Actual result

Browser CPU time
Chrome 113 - ~560ms
Chrome 113 4x slower 2633ms
Safari 16.4 - 920ms

❓ Possible solution

It appears this problem was explored some time ago and this issue was predicted. #5812

📃 Other details

attribute value
model MacBook Pro 2019
processor 2.3 Ghz 8-core i9
memory 32 GB
macOS Ventura 13.3.1

This issue is happening in our production environment with customer data.
The html used in the script above was generated to demonstrate the performance issue.

In our implementation we use @ckeditor/ckeditor5-react.
This issue is compounded by the fact that it uses Watchdog which has a _save
method that calls getData. This is throttled, but effectively pauses interaction
for the duration of the call every 5 seconds.

We also use the react CKEditor as a controlled component, storing the content and passing it
as the data prop, then updating it when the onChange callback is invoked. In addition to
us calling getData in the onchange callback, it is again called in _shouldUpdateEditor
when the react CKEditor component receives the new props.

I considered submitting this issue to the ckeditor/ckeditor5-react repo, but I think the issue
from four years ago highlights that the root cause is in getData and that any framework that
uses two way binding will suffer the same issue.

@agradl agradl added the type:bug This issue reports a buggy (incorrect) behavior. label May 24, 2023
@Witoso Witoso added the domain:performance This issue reports a problem or potential improvement regarding editor performance. label May 25, 2023
@Witoso
Copy link
Member

Witoso commented May 25, 2023

Thanks for the report. @scofalik I know you were looking into watchdog performance.

@gfu7
Copy link

gfu7 commented May 26, 2023

Recently, I have also been bothered by this similar issue. #14188
I am also paying attention to: how to use editor.getData() gracefully when as a controlled editor component.

@Witoso
Copy link
Member

Witoso commented May 26, 2023

Regarding the watchdog performance please follow the #13098.

@CKEditorBot
Copy link
Collaborator

There has been no activity on this issue for the past year. We've marked it as stale and will close it in 30 days. We understand it may still be relevant, so if you're interested in the solution, leave a comment or reaction under this issue.

@CKEditorBot
Copy link
Collaborator

We've closed your issue due to inactivity. We understand that the issue may still be relevant. If so, feel free to open a new one (and link this issue to it).

@CKEditorBot CKEditorBot added resolution:expired This issue was closed due to lack of feedback. and removed status:stale labels Jun 26, 2024
@CKEditorBot CKEditorBot closed this as not planned Won't fix, can't repro, duplicate, stale Jun 26, 2024
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. package:watchdog resolution:expired This issue was closed due to lack of feedback. type:bug This issue reports a buggy (incorrect) behavior.
Projects
None yet
Development

No branches or pull requests

5 participants