From 5f438ed39c8d559fe0ef31a2c808229a3e7beb58 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotrek=20Koszuli=C5=84ski?= Date: Wed, 8 Apr 2020 15:24:42 +0200 Subject: [PATCH 1/4] Cache the parent as it's a costly computation. --- src/model/position.js | 23 ++++++++++++++++------- 1 file changed, 16 insertions(+), 7 deletions(-) diff --git a/src/model/position.js b/src/model/position.js index 3e68adcc3..5854e7559 100644 --- a/src/model/position.js +++ b/src/model/position.js @@ -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. @@ -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 ); } /** @@ -228,17 +225,23 @@ export default class Position { * @type {module:engine/model/node~Node|null} */ get nodeAfter() { - return this.textNode === null ? this.parent.getChild( this.index ) : null; + const parent = this.parent; + const textNode = getTextNode( this, parent ); + + return textNode === null ? parent.getChild( parent.offsetToIndex( this.offset ) ) : null; } /** * 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; + const parent = this.parent; + const textNode = getTextNode( this, parent ); + + return textNode === null ? parent.getChild( parent.offsetToIndex( this.offset ) - 1 ) : null; } /** @@ -1057,3 +1060,9 @@ export default class Position { * * @typedef {String} module:engine/model/position~PositionStickiness */ + +function getTextNode( position, positionParent ) { + const node = positionParent.getChild( positionParent.offsetToIndex( position.offset ) ); + + return ( node && node.is( 'text' ) && node.startOffset < position.offset ) ? node : null; +} From 55f4a537e0de67a480bb98938b5c8c069f9de984 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotrek=20Koszuli=C5=84ski?= Date: Wed, 8 Apr 2020 15:36:08 +0200 Subject: [PATCH 2/4] Comments. --- src/model/position.js | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/model/position.js b/src/model/position.js index 5854e7559..507bc1671 100644 --- a/src/model/position.js +++ b/src/model/position.js @@ -225,6 +225,8 @@ export default class Position { * @type {module:engine/model/node~Node|null} */ get nodeAfter() { + // 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 ); @@ -238,6 +240,8 @@ export default class Position { * @type {module:engine/model/node~Node|null} */ get nodeBefore() { + // 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 ); @@ -1061,6 +1065,9 @@ 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 ) ); From 3bb2fd4d32bbb2b15a7049d4c3a2321eac21cf00 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotrek=20Koszuli=C5=84ski?= Date: Wed, 8 Apr 2020 15:52:00 +0200 Subject: [PATCH 3/4] Cache parent here too. --- src/model/position.js | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/model/position.js b/src/model/position.js index 507bc1671..f0b5624c6 100644 --- a/src/model/position.js +++ b/src/model/position.js @@ -346,10 +346,12 @@ export default class Position { * @returns {Array.} 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 } ); } } From e37cd07ddc4ebb44388f6ef1121b80b675f67fd0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotrek=20Koszuli=C5=84ski?= Date: Fri, 10 Apr 2020 20:06:38 +0200 Subject: [PATCH 4/4] Avoid ternary operators. --- src/model/position.js | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/src/model/position.js b/src/model/position.js index f0b5624c6..65accce56 100644 --- a/src/model/position.js +++ b/src/model/position.js @@ -230,7 +230,11 @@ export default class Position { const parent = this.parent; const textNode = getTextNode( this, parent ); - return textNode === null ? parent.getChild( parent.offsetToIndex( this.offset ) ) : null; + if ( textNode !== null ) { + return null; + } + + return parent.getChild( parent.offsetToIndex( this.offset ) ); } /** @@ -245,7 +249,11 @@ export default class Position { const parent = this.parent; const textNode = getTextNode( this, parent ); - return textNode === null ? parent.getChild( parent.offsetToIndex( this.offset ) - 1 ) : null; + if ( textNode !== null ) { + return null; + } + + return parent.getChild( parent.offsetToIndex( this.offset ) - 1 ); } /** @@ -1073,5 +1081,9 @@ export default class Position { function getTextNode( position, positionParent ) { const node = positionParent.getChild( positionParent.offsetToIndex( position.offset ) ); - return ( node && node.is( 'text' ) && node.startOffset < position.offset ) ? node : null; + if ( node && node.is( 'text' ) && node.startOffset < position.offset ) { + return node; + } + + return null; }