Skip to content
This repository has been archived by the owner on Jun 26, 2020. It is now read-only.

Commit

Permalink
Merge pull request #1837 from ckeditor/i/6579
Browse files Browse the repository at this point in the history
Other: Improved performance of `Position` getters (~60% gain). Reduced time of some common tasks (like loading complex content) by up to 30%. Closes ckeditor/ckeditor5#6579.
  • Loading branch information
Reinmar authored Apr 10, 2020
2 parents ff04509 + e37cd07 commit 670cd7b
Showing 1 changed file with 40 additions and 10 deletions.
50 changes: 40 additions & 10 deletions src/model/position.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
import TreeWalker from './treewalker';
import compareArrays from '@ckeditor/ckeditor5-utils/src/comparearrays';
import CKEditorError from '@ckeditor/ckeditor5-utils/src/ckeditorerror';
import Text from './text';
import { last } from 'lodash-es';

// To check if component is loaded more than once.
Expand Down Expand Up @@ -216,9 +215,7 @@ export default class Position {
* @type {module:engine/model/text~Text|null}
*/
get textNode() {
const node = this.parent.getChild( this.index );

return ( node instanceof Text && node.startOffset < this.offset ) ? node : null;
return getTextNode( this, this.parent );
}

/**
Expand All @@ -228,17 +225,35 @@ export default class Position {
* @type {module:engine/model/node~Node|null}
*/
get nodeAfter() {
return this.textNode === null ? this.parent.getChild( this.index ) : null;
// Cache parent and reuse for performance reasons. This also means that we cannot use the #index property.
// See #6579.
const parent = this.parent;
const textNode = getTextNode( this, parent );

if ( textNode !== null ) {
return null;
}

return parent.getChild( parent.offsetToIndex( this.offset ) );
}

/**
* Node directly before this position or `null` if this position is in text node.
*
* @readonly
* @type {Node}
* @type {module:engine/model/node~Node|null}
*/
get nodeBefore() {
return this.textNode === null ? this.parent.getChild( this.index - 1 ) : null;
// Cache parent and reuse for performance reasons. This also means that we cannot use the #index property.
// See #6579.
const parent = this.parent;
const textNode = getTextNode( this, parent );

if ( textNode !== null ) {
return null;
}

return parent.getChild( parent.offsetToIndex( this.offset ) - 1 );
}

/**
Expand Down Expand Up @@ -339,10 +354,12 @@ export default class Position {
* @returns {Array.<module:engine/model/item~Item>} Array with ancestors.
*/
getAncestors() {
if ( this.parent.is( 'documentFragment' ) ) {
return [ this.parent ];
const parent = this.parent;

if ( parent.is( 'documentFragment' ) ) {
return [ parent ];
} else {
return this.parent.getAncestors( { includeSelf: true } );
return parent.getAncestors( { includeSelf: true } );
}
}

Expand Down Expand Up @@ -1057,3 +1074,16 @@ export default class Position {
*
* @typedef {String} module:engine/model/position~PositionStickiness
*/

// Helper function used to inline text node access by using a cached parent.
// Reduces the access to the Position#parent property 3 times (in total, when taken into account what #nodeAfter and #nodeBefore do).
// See #6579.
function getTextNode( position, positionParent ) {
const node = positionParent.getChild( positionParent.offsetToIndex( position.offset ) );

if ( node && node.is( 'text' ) && node.startOffset < position.offset ) {
return node;
}

return null;
}

0 comments on commit 670cd7b

Please sign in to comment.