Skip to content

Reduce Position#parent use by 50%+ #6579

Closed
ckeditor/ckeditor5-engine
#1837
@Reinmar

Description

@Reinmar

📝 Provide a description of the improvement

This is a performance graph for undoing removal of the "long (semantic)" content in http://localhost:8125/ckeditor5/tests/manual/performance/setdata.html.

As you can see Position#parent is the most expensive thing here. We could not find ways to make it much faster by changing its implementation.

However, then I noticed when it's used and after looking at implementation of #nodeAfter, #nodeBefore, and #textNode I realised that we're executing the #parent getter many times per each of these prop getters calls.

You can find the current implementation of these functions here: https://github.com/ckeditor/ckeditor5-engine/blob/ff045099fdea35c2af90a6da3d1a546845aeb3ea/src/model/position.js#L199-L242

Let's see if we can save 50%+ of time by rewriting them into uglier, but more optimal forms :).


If you'd like to see this improvement implemented, add a 👍 reaction to this post.

Activity

added
type:improvementThis issue reports a possible enhancement of an existing feature.
on Apr 8, 2020
Reinmar

Reinmar commented on Apr 8, 2020

@Reinmar
MemberAuthor

After playing with the code for 15 minutes I got down to this:

From 1165ms to 378ms :).

The entire undo action got down from 3.2s to 2.2s.

PR's coming!

Reinmar

Reinmar commented on Apr 8, 2020

@Reinmar
MemberAuthor

Loading "long (semantic)" content into the editor:

  • #parent: 2300ms -> 995ms
  • entire action: 5.5s -> 4.0s
added
type:performanceThis issue reports a performance issue or a possible performance improvement.
type:refactorThis issue requires or describes a code refactoring.
on Apr 8, 2020
self-assigned this
on Apr 8, 2020
added this to the iteration 31 milestone on Apr 8, 2020
added a commit that references this issue on Apr 10, 2020

Merge pull request #1837 from ckeditor/i/6579

670cd7b
added a commit that references this issue on May 1, 2020

Merge pull request #1837 from ckeditor/i/6579

200e768
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

package:enginetype:improvementThis issue reports a possible enhancement of an existing feature.type:performanceThis issue reports a performance issue or a possible performance improvement.type:refactorThis issue requires or describes a code refactoring.

Type

No type

Projects

No projects

Relationships

None yet

    Development

    Participants

    @Reinmar

    Issue actions

      Reduce Position#parent use by 50%+ · Issue #6579 · ckeditor/ckeditor5