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

Improve mapper performance #17517

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

filipsobol
Copy link
Member

Suggested merge commit message (convention)

Other (engine): Improve performance of the getModelLength method.


While this looks like a simple change, it introduces significant performance improvements by avoiding recursion and calling this._viewToModelLengthCallbacks.get() on nodes without name or when the map is empty (most cases).

Test name Improvement in % Improvement in ms
lists -4.9% -100ms
formatting-long-paragraphs -15.0% -730ms
inline-styles -22.2% -2310ms
wiki -4.0% -140ms

We could gain even more performance by removing the public registerViewToModelLength method and the this._viewToModelLengthCallbacks map, since it's only used in legacy lists, which we are deprecating. However, it's likely that it's used by the integrators and may not be possible. This would improve the performance (on top of proposed changes) of formatting-long-paragraphs by another 180ms (-4.4%) and inline-styles by 300ms (-3.8%).

@scofalik
Copy link
Contributor

Heads up - I was going to rewrite this piece of code in a different way with my Mapper improvements. @niegowski -- do not spent time reviewing it as it may change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants