Merge pull request #17522 from ckeditor/improve-performance-of-_inser…

…tNodes

Other (engine): Improve performance of `_insertNodes`.

Other (utils): Change the implementation of `spliceArray` to modify the target array for better performance.

MINOR BREAKING CHANGE (utils): `spliceArray` now modifies the target array and doesn't accept a fourth (`count`) argument.

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

@@ -164,6 +164,8 @@ export default class NodeList implements Iterable<Node> {
 	 * @param nodes Nodes to be inserted.
 	 */
 	public _insertNodes( index: number, nodes: Iterable<Node> ): void {
+		const nodesArray: Array<Node> = [];
+
 		// Validation.
 		for ( const node of nodes ) {
 			if ( !( node instanceof Node ) ) {
@@ -174,16 +176,15 @@ export default class NodeList implements Iterable<Node> {
 				 */
 				throw new CKEditorError( 'model-nodelist-insertnodes-not-node', this );
 			}
-		}
 
-		const nodesArray = Array.from( nodes );
-		const offsetsArray = makeOffsetsArray( nodesArray );
+			nodesArray.push( node );
+		}
 
 		let offset = this.indexToOffset( index );
 
 		// Splice nodes array and offsets array into the nodelist.
-		this._nodes = spliceArray<Node>( this._nodes, nodesArray, index, 0 );
-		this._offsetToNode = spliceArray<Node>( this._offsetToNode, offsetsArray, offset, 0 );
+		spliceArray( this._nodes, nodesArray, index );
+		spliceArray( this._offsetToNode, makeOffsetsArray( nodesArray ), offset );
 
 		// Refresh indexes and offsets for nodes inside this node list. We need to do this for all inserted nodes and all nodes after them.
 		for ( let i = index; i < this._nodes.length; i++ ) {

packages/ckeditor5-utils/src/splicearray.ts

@@ -8,17 +8,15 @@
  */
 
 /**
- * Splices one array into another. To be used instead of `Array.prototype.splice` as the latter may
- * throw "Maximum call stack size exceeded" when passed huge number of items to insert.
- *
- * Note: in contrary to Array.splice, this function does not modify the original `target`.
+ * Splices one array into another. To be used instead of `Array.prototype.splice` for better
+ * performance and because the latter may throw "Maximum call stack size exceeded" error when
+ * passing huge number of items to insert.
  *
  * ```ts
- * spliceArray( [ 1, 2 ], [ 3, 4 ], 0, 0 );	// [ 3, 4, 1, 2 ]
- * spliceArray( [ 1, 2 ], [ 3, 4 ], 1, 1 );	// [ 1, 3, 4 ]
- * spliceArray( [ 1, 2 ], [ 3, 4 ], 1, 0 );	// [ 1, 3, 4, 2 ]
- * spliceArray( [ 1, 2 ], [ 3, 4 ], 2, 0 );	// [ 1, 2, 3, 4 ]
- * spliceArray( [ 1, 2 ], [],       0, 1 );	// [ 2 ]
+ * spliceArray( [ 1, 2 ], [ 3, 4 ], 0 );	// [ 3, 4, 1, 2 ]
+ * spliceArray( [ 1, 2 ], [ 3, 4 ], 1 );	// [ 1, 3, 4, 2 ]
+ * spliceArray( [ 1, 2 ], [ 3, 4 ], 2 );	// [ 1, 2, 3, 4 ]
+ * spliceArray( [ 1, 2 ], [],       0 );	// [ 1, 2 ]
  * ```
  *
  * @param target Array to be spliced.
@@ -29,28 +27,20 @@
  * @returns New spliced array.
  */
 export default function spliceArray<T>(
-	target: ReadonlyArray<T>,
-	source: ReadonlyArray<T>,
-	start: number,
-	count: number
-): Array<T> {
-	const targetLength = target.length;
-	const sourceLength = source.length;
-	const result = new Array<T>( targetLength - count + sourceLength );
-
-	let i = 0;
+	targetArray: Array<T>,
+	insertArray: Array<T>,
+	index: number
+): void {
+	const originalLength = targetArray.length;
+	const insertLength = insertArray.length;
 
-	for ( ; i < start && i < targetLength; i++ ) {
-		result[ i ] = target[ i ];
+	// Shift elements in the target array to make space for insertArray
+	for ( let i = originalLength - 1; i >= index; i-- ) {
+		targetArray[ i + insertLength ] = targetArray[ i ];
 	}
 
-	for ( let j = 0; j < sourceLength; j++, i++ ) {
-		result[ i ] = source[ j ];
+	// Copy elements from insertArray into the target array
+	for ( let i = 0; i < insertLength; i++ ) {
+		targetArray[ index + i ] = insertArray[ i ];
 	}
-
-	for ( let k = start + count; k < targetLength; k++, i++ ) {
-		result[ i ] = target[ k ];
-	}
-
-	return result;
 }

packages/ckeditor5-utils/tests/splicearray.js

@@ -11,56 +11,47 @@ describe( 'utils', () => {
 			const target = [ 1, 2 ];
 			const source = [ 3, 4 ];
 
-			const result = spliceArray( target, source, 0, 0 );
+			spliceArray( target, source, 0 );
 
-			expect( result ).to.have.members( [ 3, 4, 1, 2 ] );
+			expect( target ).to.have.members( [ 3, 4, 1, 2 ] );
 		} );
 
 		it( 'should insert elements in the middle of the target array', () => {
 			const target = [ 1, 2 ];
 			const source = [ 3, 4 ];
 
-			const result = spliceArray( target, source, 1, 0 );
+			spliceArray( target, source, 1 );
 
-			expect( result ).to.have.members( [ 1, 3, 4, 2 ] );
+			expect( target ).to.have.members( [ 1, 3, 4, 2 ] );
 		} );
 
 		it( 'should insert elements at the end of the target array', () => {
 			const target = [ 1, 2 ];
 			const source = [ 3, 4 ];
 
-			const result = spliceArray( target, source, 2, 0 );
+			spliceArray( target, source, 2 );
 
-			expect( result ).to.have.members( [ 1, 2, 3, 4 ] );
-		} );
-
-		it( 'should only splice target array when source is empty', () => {
-			const target = [ 1, 2 ];
-			const source = [];
-
-			const result = spliceArray( target, source, 0, 1 );
-
-			expect( result ).to.have.members( [ 2 ] );
+			expect( target ).to.have.members( [ 1, 2, 3, 4 ] );
 		} );
 
 		it( 'should insert elements into array which contains a large number of elements (250 000)', () => {
 			const target = 'a'.repeat( 250000 ).split( '' );
 			const source = [ 'b', 'c' ];
 			const expectedLength = target.length + source.length;
 
-			const result = spliceArray( target, source, 0, 0 );
+			spliceArray( target, source, 0 );
 
-			expect( result.length ).to.equal( expectedLength );
-			expect( result[ 0 ] ).to.equal( source[ 0 ] );
+			expect( target.length ).to.equal( expectedLength );
+			expect( target[ 0 ] ).to.equal( source[ 0 ] );
 		} );
 
 		it( 'should insert elements in the middle of the target array which contains a large number of elements (250 000)', () => {
 			const target = 'a'.repeat( 250000 ).split( '' );
 			const source = [ 'b', 'c' ];
 
-			const result = spliceArray( target, source, 5, 0 );
+			spliceArray( target, source, 5 );
 
-			expect( result[ 5 ] ).to.equal( source[ 0 ] );
+			expect( target[ 5 ] ).to.equal( source[ 0 ] );
 		} );
 	} );
 } );