Long continuous prefix move optimization

src/diff/children.js

@@ -302,16 +302,27 @@ function constructNewChildrenArray(
 				skew--;
 			} else if (matchingIndex == skewedIndex + 1) {
 				skew++;
-			} else {
-				if (matchingIndex > skewedIndex) {
-					skew--;
+			} else if (matchingIndex > skewedIndex) {
+				if (
+					matchingIndex - skewedIndex < oldChildrenLength - matchingIndex &&
+					renderResult[i + 1] != NULL &&
+					oldChildren[matchingIndex + 1] != NULL &&
+					renderResult[i + 1].key == oldChildren[matchingIndex + 1].key &&
+					renderResult[i + 1].type == oldChildren[matchingIndex + 1].type
+				) {
+					// A short prefix was moved to the end. The match starts a longer
+					// contiguous suffix, so keep that suffix in place and insert the
+					// displaced prefix when reconciliation reaches it.
+					skew += matchingIndex - skewedIndex;
 				} else {
-					skew++;
-				}
+					skew--;
 
-				// Move this VNode's DOM if the original index (matchingIndex) doesn't
-				// match the new skew index (i + new skew)
-				// In the former two branches we know that it matches after skewing
+					// Move this VNode's DOM if the original index (matchingIndex)
+					// doesn't match the new skew index (i + new skew)
+					childVNode._flags |= INSERT_VNODE;
+				}
+			} else {
+				skew++;
 				childVNode._flags |= INSERT_VNODE;
 			}
 		}

test/browser/keys.test.jsx

@@ -366,6 +366,156 @@ describe('keys', () => {
 		);
 	});
 
+	it('should not displace when the suffix after the match is shorter than the jump', () => {
+		const values = ['a', 'b', 'c', 'd', 'e', 'f', 'g', 'h', 'i', 'j'];
+
+		render(<List values={values} />, scratch);
+		expect(scratch.textContent).to.equal('abcdefghij');
+
+		// Swap two far apart children; the run following the match (j) is
+		// shorter than the jump, so the displacement path must not trigger.
+		[values[1], values[8]] = [values[8], values[1]];
+		clearLog();
+
+		render(<List values={values} />, scratch);
+		expect(scratch.textContent).to.equal('aicdefghbj');
+		expect(getLog()).to.deep.equal([
+			'<ol>abcdefghij.insertBefore(<li>i, <li>b)',
+			'<ol>aibcdefghj.insertBefore(<li>b, <li>j)'
+		]);
+	});
+
+	it('should move the shorter suffix when more than half the list is displaced', () => {
+		const values = ['a', 'b', 'c', 'd', 'e', 'f'];
+
+		render(<List values={values} />, scratch);
+		expect(scratch.textContent).to.equal('abcdef');
+
+		// Displacing 4 of 6 children: moving the displaced prefix would be more
+		// work than moving the two-child suffix, so the guard must not trigger.
+		values.push(...values.splice(0, 4));
+		clearLog();
+
+		render(<List values={values} />, scratch);
+		expect(scratch.textContent).to.equal('efabcd');
+		expect(getLog()).to.deep.equal([
+			'<ol>abcdef.insertBefore(<li>e, <li>a)',
+			'<ol>eabcdf.insertBefore(<li>f, <li>a)'
+		]);
+	});
+
+	it('should displace multiple keyed children to the end while the list grows', () => {
+		const values = ['a', 'b', 'c', 'd', 'e', 'f', 'g', 'h', 'i', 'j'];
+
+		render(<List values={values} />, scratch);
+		expect(scratch.textContent).to.equal('abcdefghij');
+
+		values.push(...values.splice(0, 3));
+		values.push('k');
+		clearLog();
+
+		render(<List values={values} />, scratch);
+		expect(scratch.textContent).to.equal('defghijabck');
+		expect(getLog()).to.deep.equal([
+			'<ol>abcdefghij.appendChild(<li>a)',
+			'<ol>bcdefghija.appendChild(<li>b)',
+			'<ol>cdefghijab.appendChild(<li>c)',
+			'<li>.appendChild(#text)',
+			'<ol>defghijabc.appendChild(<li>k)'
+		]);
+	});
+
+	it('should displace multiple keyed children to the end while another is removed', () => {
+		const values = ['a', 'b', 'c', 'd', 'e', 'f', 'g', 'h', 'i', 'j'];
+
+		render(<List values={values} />, scratch);
+		expect(scratch.textContent).to.equal('abcdefghij');
+
+		values.push(...values.splice(0, 3));
+		values.splice(values.indexOf('j'), 1);
+		clearLog();
+
+		render(<List values={values} />, scratch);
+		expect(scratch.textContent).to.equal('defghiabc');
+		expect(getLog()).to.deep.equal([
+			'<li>j.remove()',
+			'<ol>abcdefghi.appendChild(<li>a)',
+			'<ol>bcdefghia.appendChild(<li>b)',
+			'<ol>cdefghiab.appendChild(<li>c)'
+		]);
+	});
+
+	it('should displace keyed children to the end repeatedly', () => {
+		const values = ['a', 'b', 'c', 'd', 'e', 'f', 'g', 'h', 'i', 'j'];
+		const expectedDisplaceLogs = [
+			[
+				'<ol>abcdefghij.appendChild(<li>a)',
+				'<ol>bcdefghija.appendChild(<li>b)',
+				'<ol>cdefghijab.appendChild(<li>c)'
+			],
+			[
+				'<ol>defghijabc.appendChild(<li>d)',
+				'<ol>efghijabcd.appendChild(<li>e)',
+				'<ol>fghijabcde.appendChild(<li>f)'
+			],
+			[
+				'<ol>ghijabcdef.appendChild(<li>g)',
+				'<ol>hijabcdefg.appendChild(<li>h)',
+				'<ol>ijabcdefgh.appendChild(<li>i)'
+			]
+		];
+
+		render(<List values={values} />, scratch);
+		expect(scratch.textContent).to.equal('abcdefghij');
+
+		for (let n = 0; n < 3; n++) {
+			values.push(...values.splice(0, 3));
+			clearLog();
+
+			render(<List values={values} />, scratch);
+			expect(scratch.textContent).to.equal(values.join(''));
+			expect(getLog()).to.deep.equal(expectedDisplaceLogs[n], `round ${n}`);
+		}
+	});
+
+	it('should keep text siblings correct around displaced keyed children', () => {
+		// The displacement probe compares the key & type of the next pair; a raw
+		// text child passes that probe against any old text node. Ensure such a
+		// false positive can only cost extra moves, never correctness.
+		const content = condition => (
+			<ol>
+				{condition
+					? [
+							<li key="a">a</li>,
+							<li key="b">b</li>,
+							<li key="c">c</li>,
+							'mid',
+							<li key="d">d</li>,
+							<li key="e">e</li>
+						]
+					: [
+							<li key="c">c</li>,
+							'mid',
+							<li key="a">a</li>,
+							<li key="b">b</li>,
+							<li key="d">d</li>,
+							<li key="e">e</li>
+						]}
+			</ol>
+		);
+
+		render(content(true), scratch);
+		expect(scratch.innerHTML).to.equal(
+			'<ol><li>a</li><li>b</li><li>c</li>mid<li>d</li><li>e</li></ol>'
+		);
+
+		clearLog();
+		render(content(false), scratch);
+		expect(scratch.innerHTML).to.equal(
+			'<ol><li>c</li>mid<li>a</li><li>b</li><li>d</li><li>e</li></ol>'
+		);
+	});
+
 	it('should move keyed children to the end of the list', () => {
 		const values = ['a', 'b', 'c', 'd'];
 
@@ -395,6 +545,24 @@ describe('keys', () => {
 		);
 	});
 
+	it('should displace multiple keyed children to the end efficiently', () => {
+		const values = ['a', 'b', 'c', 'd', 'e', 'f', 'g', 'h', 'i', 'j'];
+
+		render(<List values={values} />, scratch);
+		expect(scratch.textContent).to.equal('abcdefghij');
+
+		values.push(...values.splice(0, 3));
+		clearLog();
+
+		render(<List values={values} />, scratch);
+		expect(scratch.textContent).to.equal('defghijabc');
+		expect(getLog()).to.deep.equal([
+			'<ol>abcdefghij.appendChild(<li>a)',
+			'<ol>bcdefghija.appendChild(<li>b)',
+			'<ol>cdefghijab.appendChild(<li>c)'
+		]);
+	});
+
 	it('should move keyed children to the beginning on longer list', () => {
 		// Preact v10 worst case
 		const values = ['a', 'b', 'c', 'd', 'e', 'f'];