feat(turbopack): wasm file write support and available_parallelism for worker pool#138
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces WASM-specific file system operations in turbo-tasks-fs and refactors turbopack-node to use a local available_parallelism module. A review comment identifies an improvement opportunity in the WASM file handling logic, suggesting a refactor to eliminate redundant matching and the use of unreachable!() to improve code clarity and safety.
| #[cfg(all(target_family = "wasm", target_os = "unknown"))] | ||
| match &*self.content { | ||
| FileContent::Content(..) => { | ||
| let create_directory = compare == FileComparison::Create; | ||
| if create_directory && let Some(parent) = full_path.parent() { | ||
| self.inner.create_directory(parent).await.with_context(|| { | ||
| format!( | ||
| "failed to create directory {parent:?} for write to \ | ||
| {full_path:?}", | ||
| ) | ||
| })?; | ||
| } | ||
| let content = self.content.clone(); | ||
| let FileContent::Content(file) = &*content else { | ||
| unreachable!() | ||
| }; | ||
| wasm_fs_offload::CLIENT | ||
| .write(&full_path, file.content().to_bytes()) | ||
| .instrument(tracing::info_span!("write file", name = ?full_path)) | ||
| .await | ||
| .with_context(|| format!("failed to write to {full_path:?}"))?; | ||
| } | ||
| FileContent::NotFound => { | ||
| wasm_fs_offload::CLIENT | ||
| .remove_file(&full_path) | ||
| .instrument(tracing::info_span!("remove file", name = ?full_path)) | ||
| .await | ||
| .with_context(|| format!("removing {full_path:?} failed"))?; | ||
| } | ||
| } |
There was a problem hiding this comment.
The WASM-specific file operation block contains redundant logic and an unnecessary double-match pattern. By cloning the content Arc once before the match, you can bind the inner file value directly in the first match arm, avoiding the need for a second let...else with an unreachable!() check. This also ensures the content remains alive across the .await points in a cleaner way.
#[cfg(all(target_family = "wasm", target_os = "unknown"))]
{
let content = self.content.clone();
match &*content {
FileContent::Content(file) => {
let create_directory = compare == FileComparison::Create;
if create_directory && let Some(parent) = full_path.parent() {
self.inner.create_directory(parent).await.with_context(|| {
format!(
"failed to create directory {parent:?} for write to \
{full_path:?}",
)
})?;
}
wasm_fs_offload::CLIENT
.write(&full_path, file.content().to_bytes())
.instrument(tracing::info_span!("write file", name = ?full_path))
.await
.with_context(|| format!("failed to write to {full_path:?}"))?;
}
FileContent::NotFound => {
wasm_fs_offload::CLIENT
.remove_file(&full_path)
.instrument(tracing::info_span!("remove file", name = ?full_path))
.await
.with_context(|| format!("removing {full_path:?} failed"))?;
}
}
}…r worker pool - turbo-tasks-fs: add wasm32-specific file write/remove via wasm_fs_offload - turbopack-node: extract available_parallelism module for cross-platform concurrency - turbopack-node: use available_parallelism() directly in evaluate pool setup
369e9b0 to
aa2e21f
Compare
Changes
turbo-tasks-fs
wasm_fs_offloadclient, gated behind#[cfg(all(target_family = "wasm", target_os = "unknown"))]turbopack-node
available_parallelisminto a dedicated module for cross-platform concurrency detectionget_evaluate_poolto useavailable_parallelism()directly instead ofavailable_parallelism().map_or(1, |v| v.get())evaluate.rs