Merge pull request #17490 from ckeditor/cc/performance-other-part-1

Other (engine): Performance improvements to StylesMap and `_removeEmptyElements`.

packages/ckeditor5-engine/src/conversion/upcastdispatcher.ts

@@ -227,9 +227,10 @@ export default class UpcastDispatcher extends /* #__PURE__ */ EmitterMixin() {
 			this._removeEmptyElements();
 
 			// Move all items that were converted in context tree to the document fragment.
-			for ( const item of Array.from( this._modelCursor.parent.getChildren() ) ) {
-				writer.append( item, documentFragment );
-			}
+			const parent = this._modelCursor.parent;
+			const children = parent._removeChildren( 0, parent.childCount );
+
+			documentFragment._insertChild( 0, children );
 
 			// Extract temporary markers elements from model and set as static markers collection.
 			( documentFragment as any ).markers = extractMarkersFromModelFragment( documentFragment, writer );
@@ -498,18 +499,24 @@ export default class UpcastDispatcher extends /* #__PURE__ */ EmitterMixin() {
 	 * as some elements might have become empty after other empty elements were removed from them.
 	 */
 	private _removeEmptyElements(): void {
-		let anyRemoved = false;
+		const toRemove = new Map<ModelElement | ModelDocumentFragment, Array<ModelElement>>();
 
 		for ( const element of this._splitParts.keys() ) {
 			if ( element.isEmpty && !this._emptyElementsToKeep.has( element ) ) {
-				this.conversionApi.writer.remove( element );
+				const children = toRemove.get( element.parent! ) || [];
+
+				children.push( element );
 				this._splitParts.delete( element );
 
-				anyRemoved = true;
+				toRemove.set( element.parent!, children );
 			}
 		}
 
-		if ( anyRemoved ) {
+		for ( const [ parent, children ] of toRemove ) {
+			parent._removeChildrenArray( children );
+		}
+
+		if ( toRemove.size ) {
 			this._removeEmptyElements();
 		}
 	}

packages/ckeditor5-engine/src/model/documentfragment.ts

@@ -341,6 +341,21 @@ export default class DocumentFragment extends TypeCheckable implements Iterable<
 		return nodes;
 	}
 
+	/**
+	 * Removes children nodes provided as an array and sets
+	 * the {@link module:engine/model/node~Node#parent parent} of these nodes to `null`.
+	 *
+	 * @internal
+	 * @param nodes Array of nodes.
+	 */
+	public _removeChildrenArray( nodes: Array<Node> ): void {
+		this._children._removeNodesArray( nodes );
+
+		for ( const node of nodes ) {
+			( node as any ).parent = null;
+		}
+	}
+
 	// @if CK_DEBUG_ENGINE // public override toString(): 'documentFragment' {
 	// @if CK_DEBUG_ENGINE // 	return 'documentFragment';
 	// @if CK_DEBUG_ENGINE // }

packages/ckeditor5-engine/src/model/element.ts

@@ -284,6 +284,21 @@ export default class Element extends Node {
 		return nodes;
 	}
 
+	/**
+	 * Removes children nodes provided as an array and sets
+	 * the {@link module:engine/model/node~Node#parent parent} of these nodes to `null`.
+	 *
+	 * @internal
+	 * @param nodes Array of nodes.
+	 */
+	public _removeChildrenArray( nodes: Array<Node> ): void {
+		this._children._removeNodesArray( nodes );
+
+		for ( const node of nodes ) {
+			( node as any ).parent = null;
+		}
+	}
+
 	/**
 	 * Creates an `Element` instance from given plain object (i.e. parsed JSON string).
 	 * Converts `Element` children to proper nodes.

packages/ckeditor5-engine/src/model/nodelist.ts

@@ -210,7 +210,6 @@ export default class NodeList implements Iterable<Node> {
 		// Remove nodes from this nodelist.
 		let offset = this.indexToOffset( indexStart );
 		const nodes = this._nodes.splice( indexStart, howMany );
-
 		const lastNode = nodes[ nodes.length - 1 ];
 		const removedOffsetSum = lastNode.startOffset! + lastNode.offsetSize - offset;
 		this._offsetToNode.splice( offset, removedOffsetSum );
@@ -231,6 +230,35 @@ export default class NodeList implements Iterable<Node> {
 		return nodes;
 	}
 
+	/**
+	 * Removes children nodes provided as an array.
+	 *
+	 * @internal
+	 * @param nodes Array of nodes.
+	 */
+	public _removeNodesArray( nodes: Array<Node> ): void {
+		if ( nodes.length == 0 ) {
+			return;
+		}
+
+		for ( const node of nodes ) {
+			node._index = null;
+			node._startOffset = null;
+		}
+
+		this._nodes = this._nodes.filter( node => node.index !== null );
+		this._offsetToNode = this._offsetToNode.filter( node => node.index !== null );
+
+		let offset = 0;
+
+		for ( let i = 0; i < this._nodes.length; i++ ) {
+			this._nodes[ i ]._index = i;
+			this._nodes[ i ]._startOffset = offset;
+
+			offset += this._nodes[ i ].offsetSize;
+		}
+	}
+
 	/**
 	 * Converts `NodeList` instance to an array containing nodes that were inserted in the node list. Nodes
 	 * are also converted to their plain object representation.

packages/ckeditor5-engine/src/model/writer.ts

@@ -231,7 +231,11 @@ export default class Writer {
 
 		const version = position.root.document ? position.root.document.version : null;
 
-		const insert = new InsertOperation( position, item, version );
+		const children = item instanceof DocumentFragment ?
+			item._removeChildren( 0, item.childCount ) :
+			item;
+
+		const insert = new InsertOperation( position, children, version );
 
 		if ( item instanceof Text ) {
 			insert.shouldReceiveAttributes = true;

packages/ckeditor5-engine/src/view/stylesmap.ts

@@ -21,6 +21,16 @@ export default class StylesMap {
 	 */
 	private _styles: Styles;
 
+	/**
+	 * Cached list of style names for faster access.
+	 */
+	private _cachedStyleNames: Array<string> | null = null;
+
+	/**
+	 * Cached list of expanded style names for faster access.
+	 */
+	private _cachedExpandedStyleNames: Array<string> | null = null;
+
 	/**
 	 * An instance of the {@link module:engine/view/stylesmap~StylesProcessor}.
 	 */
@@ -196,6 +206,9 @@ export default class StylesMap {
 	public set( styles: Styles ): void;
 
 	public set( nameOrObject: string | Styles, valueOrObject?: StyleValue ): void {
+		this._cachedStyleNames = null;
+		this._cachedExpandedStyleNames = null;
+
 		if ( isObject( nameOrObject ) ) {
 			for ( const [ key, value ] of Object.entries( nameOrObject ) ) {
 				this._styleProcessor.toNormalizedForm( key, value, this._styles );
@@ -234,6 +247,9 @@ export default class StylesMap {
 	 * @param name Style name.
 	 */
 	public remove( name: string ): void {
+		this._cachedStyleNames = null;
+		this._cachedExpandedStyleNames = null;
+
 		const path = toPath( name );
 
 		unset( this._styles, path );
@@ -406,19 +422,23 @@ export default class StylesMap {
 		}
 
 		if ( expand ) {
-			return this._styleProcessor.getStyleNames( this._styles );
+			this._cachedExpandedStyleNames = this._cachedExpandedStyleNames || this._styleProcessor.getStyleNames( this._styles );
+
+			return this._cachedExpandedStyleNames;
 		}
 
-		const entries = this.getStylesEntries();
+		this._cachedStyleNames = this._cachedStyleNames || this.getStylesEntries().map( ( [ key ] ) => key );
 
-		return entries.map( ( [ key ] ) => key );
+		return this._cachedStyleNames;
 	}
 
 	/**
 	 * Removes all styles.
 	 */
 	public clear(): void {
 		this._styles = {};
+		this._cachedStyleNames = null;
+		this._cachedExpandedStyleNames = null;
 	}
 
 	/**

packages/ckeditor5-engine/tests/model/documentfragment.js

@@ -227,6 +227,27 @@ describe( 'DocumentFragment', () => {
 		} );
 	} );
 
+	describe( '_removeChildrenArray', () => {
+		it( 'should remove children from the element', () => {
+			const _1 = new Text( '_1' );
+			const _2 = new Text( '_2' );
+			const _3 = new Text( '_3' );
+			const _4 = new Text( '_4' );
+			const _5 = new Text( '_5' );
+			const _6 = new Text( '_6' );
+
+			const frag = new DocumentFragment( [ _1, _2, _3, _4, _5, _6 ] );
+
+			frag._removeChildrenArray( [ _2, _3, _4 ] );
+
+			expect( frag.childCount ).to.equal( 3 );
+
+			expect( frag.getChild( 0 ) ).to.have.property( 'data' ).that.equals( '_1' );
+			expect( frag.getChild( 1 ) ).to.have.property( 'data' ).that.equals( '_5' );
+			expect( frag.getChild( 2 ) ).to.have.property( 'data' ).that.equals( '_6' );
+		} );
+	} );
+
 	describe( 'getChildIndex', () => {
 		it( 'should return child index', () => {
 			const frag = new DocumentFragment( [ new Element( 'p' ), new Text( 'bar' ), new Element( 'h' ) ] );

packages/ckeditor5-engine/tests/model/element.js

@@ -195,6 +195,28 @@ describe( 'Element', () => {
 		} );
 	} );
 
+	describe( '_removeChildrenArray', () => {
+		it( 'should remove children from the element', () => {
+			const _1 = new Text( '_1' );
+			const _2 = new Text( '_2' );
+			const _3 = new Text( '_3' );
+			const _4 = new Text( '_4' );
+			const _5 = new Text( '_5' );
+			const _6 = new Text( '_6' );
+
+			const element = new Element( 'elem', [], [ _1, _2, _3, _4, _5, _6 ] );
+
+			element._removeChildrenArray( [ _2, _3, _4 ] );
+
+			expect( element.childCount ).to.equal( 3 );
+			expect( element.maxOffset ).to.equal( 6 );
+
+			expect( element.getChild( 0 ) ).to.have.property( 'data' ).that.equals( '_1' );
+			expect( element.getChild( 1 ) ).to.have.property( 'data' ).that.equals( '_5' );
+			expect( element.getChild( 2 ) ).to.have.property( 'data' ).that.equals( '_6' );
+		} );
+	} );
+
 	describe( 'getNodeByPath', () => {
 		it( 'should return this node if path is empty', () => {
 			const element = new Element( 'elem' );

packages/ckeditor5-engine/tests/model/nodelist.js

@@ -253,6 +253,38 @@ describe( 'NodeList', () => {
 		} );
 	} );
 
+	describe( '_removeNodesArray', () => {
+		it( 'should remove nodes from given index and refresh index and startOffset values', () => {
+			nodes._removeNodesArray( [ foo ] );
+
+			expect( foo.index ).to.equal( null );
+			expect( foo.startOffset ).to.equal( null );
+
+			expect( nodes.length ).to.equal( 2 );
+			expect( nodes.maxOffset ).to.equal( 2 );
+
+			expect( nodes.getNode( 0 ) ).to.equal( p );
+			expect( nodes.getNode( 1 ) ).to.equal( img );
+
+			expect( nodes.getNodeIndex( p ) ).to.equal( 0 );
+			expect( nodes.getNodeIndex( img ) ).to.equal( 1 );
+			expect( p.index ).to.equal( 0 );
+			expect( img.index ).to.equal( 1 );
+
+			expect( nodes.getNodeStartOffset( p ) ).to.equal( 0 );
+			expect( nodes.getNodeStartOffset( img ) ).to.equal( 1 );
+			expect( p.startOffset ).to.equal( 0 );
+			expect( img.startOffset ).to.equal( 1 );
+		} );
+
+		it( 'should early exit when array is empty', () => {
+			nodes._removeNodesArray( [] );
+
+			expect( nodes.length ).to.equal( 3 );
+			expect( nodes.maxOffset ).to.equal( 5 );
+		} );
+	} );
+
 	describe( 'toJSON', () => {
 		it( 'should serialize empty node list', () => {
 			expect( ( new NodeList() ).toJSON() ).to.deep.equal( [] );