Explorar o código

Changed: TreeWalker optimizations, caching nodes.

Szymon Cofalik %!s(int64=10) %!d(string=hai) anos
pai
achega
8bbbcd81fe

+ 2 - 2
packages/ckeditor5-engine/src/treemodel/delta/attributedelta.js

@@ -114,8 +114,8 @@ function changeRange( doc, delta, key, value, range ) {
 
 	while ( !next.done ) {
 		// We check values only when the range contains given element, that is when the iterator "enters" the element.
-		// To prevent double-checking or not needed checking, we filter-out iterator values for ELEMENT_LEAVE position.
-		if ( next.value.type != TreeWalker.ELEMENT_LEAVE ) {
+		// To prevent double-checking or not needed checking, we filter-out iterator values for ELEMENT_END position.
+		if ( next.value.type != TreeWalker.ELEMENT_END ) {
 			valueAfter = next.value.item.attrs.getValue( key );
 
 			// At the first run of the iterator the position in undefined. We also do not have a valueBefore, but

+ 6 - 6
packages/ckeditor5-engine/src/treemodel/range.js

@@ -263,9 +263,9 @@ export default class Range {
 	 * we enter into when iterating over this range.
 	 *
 	 * **Note:** this method will not return a parent node of start position. This is in contrary to {@link treeModel.TreeWalker}
-	 * which will return that node with {@link treeModel.TreeWalker#ELEMENT_LEAVE} type. This method, also, returns each
-	 * {@link treeModel.Element} once, while iterator return it twice: for {@link treeModel.TreeWalker#ELEMENT_ENTER} and
-	 * {@link treeModel.TreeWalker#ELEMENT_LEAVE}.
+	 * which will return that node with {@link treeModel.TreeWalker#ELEMENT_END} type. This method, also, returns each
+	 * {@link treeModel.Element} once, while iterator return it twice: for {@link treeModel.TreeWalker#ELEMENT_START} and
+	 * {@link treeModel.TreeWalker#ELEMENT_END}.
 	 *
 	 * @see {treeModel.TreeWalker}
 	 * @param {Boolean} [mergeCharacters] Flag indicating whether all consecutive characters with the same attributes
@@ -280,7 +280,7 @@ export default class Range {
 		do {
 			step = it.next();
 
-			if ( step.value && step.value.type != TreeWalker.ELEMENT_LEAVE ) {
+			if ( step.value && step.value.type != TreeWalker.ELEMENT_END ) {
 				yield step.value.item;
 			}
 		} while ( !step.done );
@@ -330,9 +330,9 @@ export default class Range {
 				step = it.next();
 
 				if ( step.value ) {
-					if ( step.value.type == TreeWalker.ELEMENT_ENTER ) {
+					if ( step.value.type == TreeWalker.ELEMENT_START ) {
 						depth++;
-					} else if ( step.value.type == TreeWalker.ELEMENT_LEAVE ) {
+					} else if ( step.value.type == TreeWalker.ELEMENT_END ) {
 						depth--;
 					}
 

+ 92 - 44
packages/ckeditor5-engine/src/treemodel/treewalker.js

@@ -11,8 +11,8 @@ import Element from './element.js';
 import Position from './position.js';
 import CKEditorError from '../ckeditorerror.js';
 
-const ELEMENT_ENTER = 0;
-const ELEMENT_LEAVE = 1;
+const ELEMENT_START = 0;
+const ELEMENT_END = 1;
 const TEXT = 2;
 const CHARACTER = 3;
 
@@ -55,12 +55,30 @@ export default class TreeWalker {
 		 */
 		this.boundaries = options.boundaries ? options.boundaries : null;
 
+		/**
+		 * Start boundary cached for optimization purposes.
+		 *
+		 * @private
+		 * @property {treeModel.Element} boundaryStartParent
+		 */
+		this._boundaryStartParent = this.boundaries ? this.boundaries.start.parent : null;
+
+		/**
+		 * End boundary cached for optimization purposes.
+		 *
+		 * @private
+		 * @property {treeModel.Element} boundaryEndParent
+		 */
+		this._boundaryEndParent = this.boundaries ? this.boundaries.end.parent : null;
+
 		/**
 		 * Iterator position.
 		 *
 		 * @property {treeModel.Position} position
 		 */
-		this.position = options.position ? options.position : options.boundaries.start;
+		this.position = options.position ?
+			Position.createFromPosition( options.position ) :
+			Position.createFromPosition( options.boundaries.start );
 
 		/**
 		 * Flag indicating whether all consecutive characters with the same attributes should be
@@ -69,6 +87,14 @@ export default class TreeWalker {
 		 * @property {Boolean} mergeCharacters
 		 */
 		this.mergeCharacters = !!options.mergeCharacters;
+
+		/**
+		 * Parent of the most recently visited node. Cached for optimization purposes.
+		 *
+		 * @private
+		 * @property {treeModel.Element} visitedParent
+		 */
+		this._visitedParent = this.position.parent;
 	}
 
 	/**
@@ -77,54 +103,65 @@ export default class TreeWalker {
 	 * @returns {Object} Value between the previous and the new {@link #position}.
 	 * @returns {Boolean} return.done True if iterator is done.
 	 * @returns {Object} return.value
-	 * @returns {Number} return.value.type Encountered value type, possible options: {@link TreeWalker#ELEMENT_ENTER},
-	 * {@link TreeWalker#ELEMENT_LEAVE} or {@link TreeWalker#TEXT}.
+	 * @returns {Number} return.value.type Encountered value type, possible options: {@link TreeWalker#ELEMENT_START},
+	 * {@link TreeWalker#ELEMENT_END}, {@link TreeWalker#CHARACTER} or {@link TreeWalker#TEXT}.
 	 * @returns {treeModel.Node} return.value.node Encountered node.
 	 */
 	next() {
-		const position = this.position;
-		const parent = position.parent;
+		const position = Position.createFromPosition( this.position );
+		const parent = this._visitedParent;
 
 		// We are at the end of the root.
 		if ( parent.parent === null && position.offset === parent.getChildCount() ) {
 			return { done: true };
 		}
 
-		if ( this.boundaries && position.isEqual( this.boundaries.end ) ) {
+		// Parent can't be null so by comparing with boundaryParent we check if boundaryParent is set at all.
+		if ( parent == this._boundaryEndParent && position.offset == this.boundaries.end.offset ) {
 			return { done: true };
 		}
 
-		const nodeAfter = position.nodeAfter;
+		const node = parent.getChild( position.offset );
+
+		if ( node instanceof Element ) {
+			// Manual operations on path internals for optimization purposes. Here and in the rest of the method.
+			position.path.push( 0 );
+			this.position = position;
 
-		if ( nodeAfter instanceof Element ) {
-			this.position = Position.createFromParentAndOffset( nodeAfter, 0 );
+			this._visitedParent = node;
 
-			return formatReturnValue( ELEMENT_ENTER, nodeAfter );
-		} else if ( nodeAfter instanceof CharacterProxy ) {
+			return formatReturnValue( ELEMENT_START, node );
+		} else if ( node instanceof CharacterProxy ) {
 			if ( this.mergeCharacters ) {
-				let charactersCount = nodeAfter._nodeListText.text.length - nodeAfter._index;
+				let charactersCount = node._nodeListText.text.length - node._index;
 				let offset = position.offset + charactersCount;
 
-				if ( this.boundaries && this.boundaries.end.parent == parent && this.boundaries.end.offset < offset ) {
+				if ( this._boundaryEndParent == parent && this.boundaries.end.offset < offset ) {
 					offset = this.boundaries.end.offset;
 					charactersCount = offset - position.offset;
 				}
 
-				let text = nodeAfter._nodeListText.text.substr( nodeAfter._index, charactersCount );
+				let text = node._nodeListText.text.substr( node._index, charactersCount );
 				let textFragment = new TextFragment( position, text );
 
-				this.position = Position.createFromParentAndOffset( parent, offset );
+				position.offset = offset;
+				this.position = position;
 
 				return formatReturnValue( TEXT, textFragment );
 			} else {
-				this.position = Position.createFromParentAndOffset( parent, position.offset + 1 );
+				position.offset++;
+				this.position = position;
 
-				return formatReturnValue( CHARACTER, nodeAfter );
+				return formatReturnValue( CHARACTER, node );
 			}
 		} else {
-			this.position = Position.createFromParentAndOffset( parent.parent, parent.getIndex() + 1 );
+			position.path.pop();
+			position.offset++;
+			this.position = position;
 
-			return formatReturnValue( ELEMENT_LEAVE, this.position.nodeBefore );
+			this._visitedParent = parent.parent;
+
+			return formatReturnValue( ELEMENT_END, parent );
 		}
 	}
 
@@ -134,55 +171,66 @@ export default class TreeWalker {
 	 * @returns {Object} Value between the previous and the new {@link #position}.
 	 * @returns {Boolean} return.done True if iterator is done.
 	 * @returns {Object} return.value
-	 * @returns {Number} return.value.type Encountered value type, possible options: {@link TreeWalker#ELEMENT_ENTER},
-	 * {@link TreeWalker#ELEMENT_LEAVE} or {@link TreeWalker#TEXT}.
+	 * @returns {Number} return.value.type Encountered value type, possible options: {@link TreeWalker#ELEMENT_START},
+	 * {@link TreeWalker#ELEMENT_END}, {@link TreeWalker#CHARACTER} or {@link TreeWalker#TEXT}.
 	 * @returns {treeModel.Node} return.value.item Scanned node.
 	 */
 	previous() {
-		const position = this.position;
-		const parent = position.parent;
+		const position = Position.createFromPosition( this.position );
+		const parent = this._visitedParent;
 
-		// We are at the beginning of the root.
+		// We are at the end of the root.
 		if ( parent.parent === null && position.offset === 0 ) {
 			return { done: true };
 		}
 
-		if ( this.boundaries && position.isEqual( this.boundaries.start ) ) {
+		// Parent can't be null so by comparing with boundaryParent we check if boundaryParent is set at all.
+		if ( parent == this._boundaryStartParent && position.offset == this.boundaries.start.offset ) {
 			return { done: true };
 		}
 
-		const nodeBefore = position.nodeBefore;
+		const node = parent.getChild( position.offset - 1 );
 
-		if ( nodeBefore instanceof Element ) {
-			this.position = Position.createFromParentAndOffset( nodeBefore, nodeBefore.getChildCount() );
+		if ( node instanceof Element ) {
+			// Manual operations on path internals for optimization purposes. Here and in the rest of the method.
+			position.offset--;
+			position.path.push( node.getChildCount() );
+			this.position = position;
 
-			return formatReturnValue( ELEMENT_LEAVE, nodeBefore );
-		} else if ( nodeBefore instanceof CharacterProxy ) {
+			this._visitedParent = node;
+
+			return formatReturnValue( ELEMENT_END, node );
+		} else if ( node instanceof CharacterProxy ) {
 			if ( this.mergeCharacters ) {
-				let charactersCount = nodeBefore._index + 1;
+				let charactersCount = node._index + 1;
 				let offset = position.offset - charactersCount;
 
-				if ( this.boundaries && this.boundaries.start.parent == parent && this.boundaries.start.offset > offset ) {
+				if ( this._boundaryStartParent == parent && this.boundaries.start.offset > offset ) {
 					offset = this.boundaries.start.offset;
 					charactersCount = position.offset - offset;
 				}
 
-				let text = nodeBefore._nodeListText.text.substr( nodeBefore._index + 1 - charactersCount, charactersCount );
+				let text = node._nodeListText.text.substr( node._index + 1 - charactersCount, charactersCount );
 
-				this.position = Position.createFromParentAndOffset( parent, offset );
+				position.offset = offset;
+				this.position = position;
 
 				let textFragment = new TextFragment( this.position, text );
 
 				return formatReturnValue( TEXT, textFragment );
 			} else {
-				this.position = Position.createFromParentAndOffset( parent, position.offset - 1 );
+				position.offset--;
+				this.position = position;
 
-				return formatReturnValue( CHARACTER, nodeBefore );
+				return formatReturnValue( CHARACTER, node );
 			}
 		} else {
-			this.position = Position.createFromParentAndOffset( parent.parent, parent.getIndex() );
+			position.path.pop();
+			this.position = position;
+
+			this._visitedParent = parent.parent;
 
-			return formatReturnValue( ELEMENT_ENTER, this.position.nodeAfter );
+			return formatReturnValue( ELEMENT_START, parent );
 		}
 	}
 }
@@ -198,22 +246,22 @@ function formatReturnValue( type, item ) {
 }
 
 /**
- * Flag for entering element.
+ * Flag for encountering start of an element.
  *
  * @static
  * @readonly
  * @property {Number}
  */
-TreeWalker.ELEMENT_ENTER = ELEMENT_ENTER;
+TreeWalker.ELEMENT_START = ELEMENT_START;
 
 /**
- * Flag for leaving element.
+ * Flag for encountering end of an element.
  *
  * @static
  * @readonly
  * @property {Number}
  */
-TreeWalker.ELEMENT_LEAVE = ELEMENT_LEAVE;
+TreeWalker.ELEMENT_END = ELEMENT_END;
 
 /**
  * Flag for text.

+ 16 - 16
packages/ckeditor5-engine/tests/treemodel/treewalker.js

@@ -17,14 +17,14 @@ import Range from '/ckeditor5/core/treemodel/range.js';
 import CKEditorError from '/ckeditor5/core/ckeditorerror.js';
 
 describe( 'range iterator', () => {
-	let ELEMENT_ENTER, ELEMENT_LEAVE, CHARACTER, TEXT;
+	let ELEMENT_START, ELEMENT_END, CHARACTER, TEXT;
 
 	let doc, expectedItems, expectedItemsMerged, root, img1, paragraph, b, a, r, img2, x;
 
 	before( () => {
-		ELEMENT_ENTER = PositionIterator.ELEMENT_ENTER;
-		ELEMENT_LEAVE = PositionIterator.ELEMENT_LEAVE;
-		TEXT = PositionIterator.TEXT;
+		ELEMENT_START = TreeWalker.ELEMENT_START;
+		ELEMENT_END = TreeWalker.ELEMENT_END;
+		TEXT = TreeWalker.TEXT;
 
 		doc = new Document();
 		root = doc.createRoot( 'root' );
@@ -54,28 +54,28 @@ describe( 'range iterator', () => {
 		root.insertChildren( 0, [ img1, paragraph ] );
 
 		expectedItems = [
-			{ type: ELEMENT_ENTER, item: img1 },
-			{ type: ELEMENT_LEAVE, item: img1 },
-			{ type: ELEMENT_ENTER, item: paragraph },
+			{ type: ELEMENT_START, item: img1 },
+			{ type: ELEMENT_END, item: img1 },
+			{ type: ELEMENT_START, item: paragraph },
 			{ type: CHARACTER, text: 'b', attrs: [ attrBoldTrue ] },
 			{ type: CHARACTER, text: 'a', attrs: [ attrBoldTrue ] },
 			{ type: CHARACTER, text: 'r', attrs: [] },
-			{ type: ELEMENT_ENTER, item: img2 },
-			{ type: ELEMENT_LEAVE, item: img2 },
+			{ type: ELEMENT_START, item: img2 },
+			{ type: ELEMENT_END, item: img2 },
 			{ type: CHARACTER, text: 'x', attrs: [] },
-			{ type: ELEMENT_LEAVE, item: paragraph }
+			{ type: ELEMENT_END, item: paragraph }
 		];
 
 		expectedItemsMerged = [
-			{ type: ELEMENT_ENTER, item: img1 },
-			{ type: ELEMENT_LEAVE, item: img1 },
-			{ type: ELEMENT_ENTER, item: paragraph },
+			{ type: ELEMENT_START, item: img1 },
+			{ type: ELEMENT_END, item: img1 },
+			{ type: ELEMENT_START, item: paragraph },
 			{ type: TEXT, text: 'ba', attrs: [ attrBoldTrue ] },
 			{ type: TEXT, text: 'r', attrs: [] },
-			{ type: ELEMENT_ENTER, item: img2 },
-			{ type: ELEMENT_LEAVE, item: img2 },
+			{ type: ELEMENT_START, item: img2 },
+			{ type: ELEMENT_END, item: img2 },
 			{ type: TEXT, text: 'x', attrs: [] },
-			{ type: ELEMENT_LEAVE, item: paragraph }
+			{ type: ELEMENT_END, item: paragraph }
 		];
 	} );