Performance: Parallelize recursive directory copying

[ivan-ottinger]

Related issues

• Related to STU-980

Proposed Changes

• parallelize recursive directory copying

Compared to the current trunk I have measured about 16% speed-up of the site creation process on macOS (about 1.2 s saving). On Windows the percentage is smaller (about 6%) - as the overall site creation process takes longer there.

Testing Instructions

1. Check out the PR branch and build the app with npm install && npm start.
2. Create several new sites.
3. The process should work correctly, the sites should get created and run without any issues.

Pre-merge Checklist

• Have you checked for TypeScript, React or other console errors?

[fredrikekelund]

Clever solution, @ivan-ottinger 👍 We should add p-limit@^3.1.0 to our package.json file before landing this PR, but I'll still approve it now to unblock you.

I went on a tangent exploring how we might use the limit.map API which is available in more recent version of p-limit, but ended up finding that it actually breaks in several ways (see my review comment).

[fredrikekelund]

I was going to suggest we do something like my suggestion below, but apparently, there are several issues with this solution (see bottom):

const recursiveCopyLimiter = pLimit( 100 );

export async function recursiveCopyDirectory(
	source: string,
	destination: string
): Promise< void > {
	await fsPromises.mkdir( destination, { recursive: true } );
	const entries = await fsPromises.readdir( source, { withFileTypes: true } );

	await recursiveCopyLimiter.map( entries, async ( entry ) => {
		const sourcePath = path.join( source, entry.name );
		const destinationPath = path.join( destination, entry.name );

		if ( entry.isDirectory() ) {
			await recursiveCopyDirectory( sourcePath, destinationPath );
		} else if ( entry.isFile() ) {
			await fsPromises.copyFile( sourcePath, destinationPath );
		}
	} );
}

• We should add p-limit to package.json, but something with how the more recent versions use ESM doesn't play nicely with our Vite config, and I end up getting errors like "pLimit is not a function" when testing my suggestion. Frustrating…
• I thought that moving the limiter definition outside the function to control the overall parallelism was smart, but that won't work (see Recursive use of p-limit may cause silent task loss when inner tasks depend on limited concurrency sindresorhus/p-limit#91)

The solution to these problems, as I see it, is to leave the code as is and add p-limit@^3.1.0 to our package.json file.

[fredrikekelund]

Out of curiosity, did you compare performance between this approach and using the copy function from the fs-extra module, @ivan-ottinger?

We're using that API elsewhere in our codebase. Might be nice to standardize on the most performant approach.

[wpmobilebot]

📊 Performance Test Results

Comparing d13094b vs trunk

site-editor

Metric	trunk	d13094b	Diff	Change
load	2832.00 ms	2839.00 ms	+7.00 ms	🔴 0.2%

site-startup

Metric	trunk	d13094b	Diff	Change
siteCreation	7091.00 ms	8088.00 ms	+997.00 ms	🔴 14.1%
siteStartup	3941.00 ms	3946.00 ms	+5.00 ms	🔴 0.1%

---

Results are median values from multiple test runs.

Legend: 🟢 Improvement (faster) | 🔴 Regression (slower) | ⚪ No change

[ivan-ottinger]

Out of curiosity, did you compare performance between this approach and using the copy function from the fs-extra module, @ivan-ottinger?

We're using that API elsewhere in our codebase. Might be nice to standardize on the most performant approach.

Good point! I have compared all three approaches on macOS. Using current p-limit implamentation is slightly faster than the copy function from the fs-extra module. But the difference is quite small. Because of that I think it is worth switching to the copy function. The code will be more maintainable and we don't need to import p-limit.

→ I have made the change in fbb9352.

[fredrikekelund]

LGTM 👍 Thanks for taking another look and improving consistency.

[gcsecsey]

Thanks @ivan-ottinger for improving the performance! The changes LGTM, and I found no regressions when testing both on Mac and Windows. 👍