codemirror / codemirror5

In-browser code editor (version 5, legacy)
http://codemirror.net/5/
MIT License
26.82k stars 4.97k forks source link

Non-linear performance with 1000 CodeMirrors on a single page #6462

Open echarles opened 3 years ago

echarles commented 3 years ago

JupyterLab notebook is using CodeMirrors (https://github.com/jupyterlab/jupyterlab/tree/master/packages/codemirror-extension) as default editor. Some users with large notebooks have issues with performance and we are working to resolve those.

We have done a few experiments adding 1000 codemirror on a single page (https://github.com/jupyterlab/benchmarks/blob/6f6878193c34be3d04385b49db484189e00cf79e/experiments/codemirror/src/index.tsx#L10-L26) and have seen exponential growth in time for each code mirror we are adding. We have used the contain css class to mitigate this, but still we think something could be done to have a linear time.

codemirror-contain

You can read more context on this deck https://jupyterlab-benchmarks-root-causes.echarles.vercel.app

Any tought on this will be very helpful to the JupyterLab community. If we can identify a fix, I am happy to open a PR on the CodeMirror code base.

jasongrout commented 3 years ago

One thought is: Are there some expensive calculations (measuring sizes, for example?) where the results could be shared among many CodeMirror instances on a page that we know have very similar construction? (I haven't explored this idea fully...)

marijnh commented 3 years ago

That.... doesn't look exponential by any means. And given that the CSS contain hack flattens it to almost linear, I'm not sure what you expect CodeMirror to be able to do about this.

Since each editor takes its layout from the DOM, and has different content and potentially different styling, I don't see any meaningful way in which size calculations could be shared between editors.

echarles commented 3 years ago

@marijnh Sorry, I have been using Exponential to highlight it is not linear. Here is another plot which may better show what I was referring to.

codemirrors

In that plot, I would expect 1000 Code Mirror to render after e.g. 10000 unit of time, and it renders after 40000 unit of time, so a penality of 400%.

echarles commented 3 years ago

@marijnh I did the same experiment with ProseMirror, and it does not show that issue (the 1000 editors are rendered super fast).

marijnh commented 3 years ago

It's indeed superlinear—I guess it could be exponential with some base very close to 1, but more likely it's merely quadratic.

I recently ran into a similar issue in CodeMirror 6, where it turned out to be related to document.createRange 'leaking' some kind of stateful object that keeps getting updated as the document is further changed. You could try to set up uses of the range function in the codebase to call detach after they are done with their range, and see if that helps.

echarles commented 3 years ago

Hi @marijnh I have renamed this issue to Non linear... to better reflect the behavior. So we want something like y=x instead of y=f(x) (whatever f(x) is). And thank you for all the work you do on codemirror to e.g. power scientist notebook lik JupyterLab.

My intuition was driving me to look at the heigh resize that would generate a forced layout of the complete page. The more editors on the page, the more time the browser would need to repaint the complete page. I will also look at the the range which would need to be detached. My next step if to dig into the codemirror source to experiment and debug in those 2 areas (force layout due to height resize + memory leaks due to non detached ranges). You refer to CodeMirror 6: are you meaning master branch (I see only version 5 is released).

What do you think of the prosemirror behavior which does renders much faster. Is there something we can learn from the following image comparing 1000 editor for various editors?

exp

marijnh commented 3 years ago

You refer to CodeMirror 6: are you meaning master branch (I see only version 5 is released).

It's released to npm under @codemirror/next (but the API isn't stable yet).

What do you think of the prosemirror behavior which does renders much faster.

ProseMirror does less work on init—it uses the DOM more directly, so it doesn't, for example, need to force a layout on initialization.

Have you tried wrapping the editor initialization in an operation? This is a bit awkward, since you need to initialize the first editor to be able to call that method at all, but it might cut down on the DOM reflows and thus help with performance.

echarles commented 3 years ago

ProseMirror does less work on init—it uses the DOM more directly, so it doesn't, for example, need to force a layout on initialization.

So CodeMirror forces a layout on initialization, which maybe the simple explanation to that non-linear behavior (the more on the page, the more time the browser need to relayout its complete page). My understanding is that a forced layout always applies to the entire page content. Is this also your understanding?

Side question: would it make sense one day to migrate to ProseMirror? So far I have seen ProseMirror not targetting the same use cases as CodeMirror (we need in JupyterLab to have language syntax and code autocomplete).

Have you tried wrapping the editor initialization in an operation? This is a bit awkward, since you need to initialize the first editor to be able to call that method at all, but it might cut down on the DOM reflows and thus help with performance.

I can try but will need to better understand how to do that. You say: initiliaze the first one normally then wrap the 999 others initialization into an cm.operation(...)? I guess I need to invoke startOperation, the run the 999 operation and then invoke endOperation?

Would this kind of implementation bring better behavior than the hack I have mentioned in the first comment: Use contain css class set to Strict, Add the 1000 CodeMirrors, and set contain=None?

echarles commented 3 years ago

I have the following figures with the operation method (the last step show remaning time is needed to render the 1000 codemirrors, see the spike as the right of the plot for the Operation and Operation+Contain flavors). I will now have a look at @codemirror/next but as the API has changed, I will need to find my way.

codemirror-comparison

echarles commented 3 years ago

This was a quick one... Now same graph with vanilla codemirror/next which is the fastest. @marijnh is codemirror/next backwards compatible?

codemirror-comparison

marijnh commented 3 years ago

It's not backwards compatible with version 5, as I guess you noticed, and it isn't stable yet either (though it's stabilizing, and won't change all over the place anymore).

echarles commented 3 years ago

It's not backwards compatible with version 5, as I guess you noticed,

Yep, have seen that, and guessed also that the plan was not to make it backward compatible. I was just wondering how difficult it would be to migrate?

steveluscher commented 3 years ago

Are there some expensive calculations (measuring sizes, for example?)

@jasongrout, related: #6470.