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

Commit 670cd7b

Browse files
authored
Merge pull request #1837 from ckeditor/i/6579
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.
2 parents ff04509 + e37cd07 commit 670cd7b

File tree

1 file changed

+40
-10
lines changed

1 file changed

+40
-10
lines changed

src/model/position.js

Lines changed: 40 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@
1010
import TreeWalker from './treewalker';
1111
import compareArrays from '@ckeditor/ckeditor5-utils/src/comparearrays';
1212
import CKEditorError from '@ckeditor/ckeditor5-utils/src/ckeditorerror';
13-
import Text from './text';
1413
import { last } from 'lodash-es';
1514

1615
// To check if component is loaded more than once.
@@ -216,9 +215,7 @@ export default class Position {
216215
* @type {module:engine/model/text~Text|null}
217216
*/
218217
get textNode() {
219-
const node = this.parent.getChild( this.index );
220-
221-
return ( node instanceof Text && node.startOffset < this.offset ) ? node : null;
218+
return getTextNode( this, this.parent );
222219
}
223220

224221
/**
@@ -228,17 +225,35 @@ export default class Position {
228225
* @type {module:engine/model/node~Node|null}
229226
*/
230227
get nodeAfter() {
231-
return this.textNode === null ? this.parent.getChild( this.index ) : null;
228+
// Cache parent and reuse for performance reasons. This also means that we cannot use the #index property.
229+
// See #6579.
230+
const parent = this.parent;
231+
const textNode = getTextNode( this, parent );
232+
233+
if ( textNode !== null ) {
234+
return null;
235+
}
236+
237+
return parent.getChild( parent.offsetToIndex( this.offset ) );
232238
}
233239

234240
/**
235241
* Node directly before this position or `null` if this position is in text node.
236242
*
237243
* @readonly
238-
* @type {Node}
244+
* @type {module:engine/model/node~Node|null}
239245
*/
240246
get nodeBefore() {
241-
return this.textNode === null ? this.parent.getChild( this.index - 1 ) : null;
247+
// Cache parent and reuse for performance reasons. This also means that we cannot use the #index property.
248+
// See #6579.
249+
const parent = this.parent;
250+
const textNode = getTextNode( this, parent );
251+
252+
if ( textNode !== null ) {
253+
return null;
254+
}
255+
256+
return parent.getChild( parent.offsetToIndex( this.offset ) - 1 );
242257
}
243258

244259
/**
@@ -339,10 +354,12 @@ export default class Position {
339354
* @returns {Array.<module:engine/model/item~Item>} Array with ancestors.
340355
*/
341356
getAncestors() {
342-
if ( this.parent.is( 'documentFragment' ) ) {
343-
return [ this.parent ];
357+
const parent = this.parent;
358+
359+
if ( parent.is( 'documentFragment' ) ) {
360+
return [ parent ];
344361
} else {
345-
return this.parent.getAncestors( { includeSelf: true } );
362+
return parent.getAncestors( { includeSelf: true } );
346363
}
347364
}
348365

@@ -1057,3 +1074,16 @@ export default class Position {
10571074
*
10581075
* @typedef {String} module:engine/model/position~PositionStickiness
10591076
*/
1077+
1078+
// Helper function used to inline text node access by using a cached parent.
1079+
// Reduces the access to the Position#parent property 3 times (in total, when taken into account what #nodeAfter and #nodeBefore do).
1080+
// See #6579.
1081+
function getTextNode( position, positionParent ) {
1082+
const node = positionParent.getChild( positionParent.offsetToIndex( position.offset ) );
1083+
1084+
if ( node && node.is( 'text' ) && node.startOffset < position.offset ) {
1085+
return node;
1086+
}
1087+
1088+
return null;
1089+
}

0 commit comments

Comments
 (0)