Skip to content

Commit

Permalink
Revert "Reduce the number of tasks for better performance" (#5953)
Browse files Browse the repository at this point in the history
Reverts #5950

Closes WEB-1569
  • Loading branch information
sokra authored Sep 14, 2023
1 parent eeb0b56 commit c39c93d
Show file tree
Hide file tree
Showing 5 changed files with 97 additions and 93 deletions.
56 changes: 32 additions & 24 deletions crates/turbo-tasks-fs/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1300,16 +1300,19 @@ pub async fn rebase(
Ok(new_base.fs.root().join(new_path))
}

// Not turbo-tasks functions, only delegating
#[turbo_tasks::value_impl]
impl FileSystemPath {
pub fn read(self: Vc<Self>) -> Vc<FileContent> {
#[turbo_tasks::function]
pub async fn read(self: Vc<Self>) -> Vc<FileContent> {
self.fs().read(self)
}

pub fn read_link(self: Vc<Self>) -> Vc<LinkContent> {
#[turbo_tasks::function]
pub async fn read_link(self: Vc<Self>) -> Vc<LinkContent> {
self.fs().read_link(self)
}

#[turbo_tasks::function]
pub fn read_json(self: Vc<Self>) -> Vc<FileJsonContent> {
self.fs().read(self).parse_json()
}
Expand All @@ -1318,41 +1321,26 @@ impl FileSystemPath {
///
/// DETERMINISM: Result is in random order. Either sort result or do not
/// depend on the order.
pub fn read_dir(self: Vc<Self>) -> Vc<DirectoryContent> {
#[turbo_tasks::function]
pub async fn read_dir(self: Vc<Self>) -> Vc<DirectoryContent> {
self.fs().read_dir(self)
}

pub fn track(self: Vc<Self>) -> Vc<Completion> {
#[turbo_tasks::function]
pub async fn track(self: Vc<Self>) -> Vc<Completion> {
self.fs().track(self)
}

#[turbo_tasks::function]
pub fn write(self: Vc<Self>, content: Vc<FileContent>) -> Vc<Completion> {
self.fs().write(self, content)
}

#[turbo_tasks::function]
pub fn write_link(self: Vc<Self>, target: Vc<LinkContent>) -> Vc<Completion> {
self.fs().write_link(self, target)
}

pub fn metadata(self: Vc<Self>) -> Vc<FileMeta> {
self.fs().metadata(self)
}

pub fn realpath(self: Vc<Self>) -> Vc<FileSystemPath> {
self.realpath_with_links().path()
}

pub fn rebase(
fs_path: Vc<FileSystemPath>,
old_base: Vc<FileSystemPath>,
new_base: Vc<FileSystemPath>,
) -> Vc<FileSystemPath> {
rebase(fs_path, old_base, new_base)
}
}

#[turbo_tasks::value_impl]
impl FileSystemPath {
#[turbo_tasks::function]
pub async fn parent(self: Vc<Self>) -> Result<Vc<FileSystemPath>> {
let this = self.await?;
Expand All @@ -1367,6 +1355,11 @@ impl FileSystemPath {
Ok(FileSystemPath::new_normalized(this.fs, p))
}

#[turbo_tasks::function]
pub fn metadata(self: Vc<Self>) -> Vc<FileMeta> {
self.fs().metadata(self)
}

#[turbo_tasks::function]
// It is important that get_type uses read_dir and not stat/metadata.
// - `get_type` is called very very often during resolving and stat would
Expand Down Expand Up @@ -1398,6 +1391,11 @@ impl FileSystemPath {
}
}

#[turbo_tasks::function]
pub fn realpath(self: Vc<Self>) -> Vc<FileSystemPath> {
self.realpath_with_links().path()
}

#[turbo_tasks::function]
pub async fn realpath_with_links(self: Vc<Self>) -> Result<Vc<RealPathResult>> {
let this = self.await?;
Expand Down Expand Up @@ -1444,6 +1442,16 @@ impl FileSystemPath {
}
}

impl FileSystemPath {
pub fn rebase(
fs_path: Vc<FileSystemPath>,
old_base: Vc<FileSystemPath>,
new_base: Vc<FileSystemPath>,
) -> Vc<FileSystemPath> {
rebase(fs_path, old_base, new_base)
}
}

#[turbo_tasks::value_impl]
impl ValueToString for FileSystemPath {
#[turbo_tasks::function]
Expand Down
7 changes: 2 additions & 5 deletions crates/turbopack-core/src/issue/resolve.rs
Original file line number Diff line number Diff line change
Expand Up @@ -81,12 +81,9 @@ impl Issue for ResolvingIssue {
request_type = self.request_type,
)?;
if let Some(import_map) = &self.resolve_options.await?.import_map {
let result = import_map
.await?
.lookup(self.file_path, self.request)
.await?;
let result = import_map.lookup(self.file_path, self.request);

match result.cell().to_string().await {
match result.to_string().await {
Ok(str) => writeln!(detail, "Import map: {}", str)?,
Err(err) => {
writeln!(
Expand Down
82 changes: 36 additions & 46 deletions crates/turbopack-core/src/resolve/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1010,7 +1010,7 @@ pub async fn resolve(
request: Vc<Request>,
options: Vc<ResolveOptions>,
) -> Result<Vc<ResolveResult>> {
let raw_result = resolve_internal(lookup_path, request, options).await?;
let raw_result = resolve_internal(lookup_path, request, options);
let result = handle_resolve_plugins(lookup_path, request, options, raw_result);
Ok(result)
}
Expand Down Expand Up @@ -1080,15 +1080,7 @@ async fn handle_resolve_plugins(
.cell())
}

fn resolve_internal_boxed(
lookup_path: Vc<FileSystemPath>,
request: Vc<Request>,
options: Vc<ResolveOptions>,
) -> Pin<Box<dyn Future<Output = Result<Vc<ResolveResult>>> + Send>> {
Box::pin(resolve_internal(lookup_path, request, options))
}

// Not a turbo-tasks function: Called too often
#[turbo_tasks::function]
async fn resolve_internal(
lookup_path: Vc<FileSystemPath>,
request: Vc<Request>,
Expand All @@ -1100,10 +1092,11 @@ async fn resolve_internal(

// Apply import mappings if provided
if let Some(import_map) = &options_value.import_map {
let result = import_map.await?.lookup(lookup_path, request).await?;
let result_ref = import_map.lookup(lookup_path, request).await?;
let result = &*result_ref;
if !matches!(result, ImportMapResult::NoEntry) {
let resolved_result = resolve_import_map_result(
&result,
result,
lookup_path,
lookup_path,
request,
Expand All @@ -1129,9 +1122,8 @@ async fn resolve_internal(
Request::Alternatives { requests } => {
let results = requests
.iter()
.map(|req| resolve_internal_boxed(lookup_path, *req, options))
.try_join()
.await?;
.map(|req| resolve_internal(lookup_path, *req, options))
.collect();

merge_results(results)
}
Expand Down Expand Up @@ -1180,17 +1172,14 @@ async fn resolve_internal(
// This ensures the order of the patterns (extensions) is
// preserved, `Pattern::Alternatives` inside a `Request::Raw` does not preserve
// the order
let results = patterns
.into_iter()
.map(|pattern| {
resolve_internal_boxed(
lookup_path,
Request::raw(Value::new(pattern), *query, *force_in_lookup_dir),
options,
)
})
.try_join()
.await?;
let mut results = Vec::new();
for pattern in patterns {
results.push(resolve_internal(
lookup_path,
Request::raw(Value::new(pattern), *query, *force_in_lookup_dir),
options,
));
}

merge_results(results)
}
Expand Down Expand Up @@ -1223,7 +1212,7 @@ async fn resolve_internal(
.cell()
.emit();

resolve_internal_boxed(lookup_path.root(), relative, options).await?
resolve_internal(lookup_path.root(), relative, options)
}
Request::Windows { path: _, query: _ } => {
ResolvingIssue {
Expand Down Expand Up @@ -1291,9 +1280,10 @@ async fn resolve_internal(
// Apply fallback import mappings if provided
if let Some(import_map) = &options_value.fallback_import_map {
if *result.is_unresolveable().await? {
let result = import_map.await?.lookup(lookup_path, request).await?;
let result_ref = import_map.lookup(lookup_path, request).await?;
let result = &*result_ref;
let resolved_result = resolve_import_map_result(
&result,
result,
lookup_path,
lookup_path,
request,
Expand Down Expand Up @@ -1328,17 +1318,15 @@ async fn resolve_into_folder(
)
})?;
let request = Request::parse(Value::new(str.into()));
return resolve_internal_boxed(package_path, request, options).await;
return Ok(resolve_internal(package_path, request, options));
}
ResolveIntoPackage::MainField(name) => {
if let Some(package_json) = &*read_package_json(package_json_path).await? {
if let Some(field_value) = package_json[name].as_str() {
let request =
Request::parse(Value::new(normalize_request(field_value).into()));

let resolved =
resolve_internal_boxed(package_path, request, options).await?;
let result = &*resolved.await?;
let result = &*resolve_internal(package_path, request, options).await?;
// we are not that strict when a main field fails to resolve
// we continue to try other alternatives
if !result.is_unresolveable_ref() {
Expand Down Expand Up @@ -1512,7 +1500,7 @@ async fn resolve_module_request(
new_pat.push_front(".".to_string().into());

let relative = Request::relative(Value::new(new_pat), query, true);
results.push(resolve_internal_boxed(*package_path, relative, options).await?);
results.push(resolve_internal(*package_path, relative, options));
}

Ok(merge_results_with_affecting_sources(
Expand Down Expand Up @@ -1540,7 +1528,7 @@ async fn resolve_import_map_result(
{
None
} else {
Some(resolve_internal_boxed(lookup_path, request, options).await?)
Some(resolve_internal(lookup_path, request, options))
}
}
ImportMapResult::Alternatives(list) => {
Expand Down Expand Up @@ -1577,14 +1565,17 @@ fn resolve_import_map_result_boxed<'a>(
options: Vc<ResolveOptions>,
query: Vc<String>,
) -> Pin<Box<dyn Future<Output = ResolveImportMapResult> + Send + 'a>> {
Box::pin(resolve_import_map_result(
result,
lookup_path,
original_lookup_path,
original_request,
options,
query,
))
Box::pin(async move {
resolve_import_map_result(
result,
lookup_path,
original_lookup_path,
original_request,
options,
query,
)
.await
})
}

async fn resolve_alias_field_result(
Expand All @@ -1604,12 +1595,11 @@ async fn resolve_alias_field_result(
}

if let Some(value) = result.as_str() {
return Ok(resolve_internal_boxed(
return Ok(resolve_internal(
package_path,
Request::parse(Value::new(Pattern::Constant(value.to_string()))).with_query(query),
resolve_options,
)
.await?
.with_affecting_sources(refs));
}

Expand Down Expand Up @@ -1756,7 +1746,7 @@ async fn handle_exports_imports_field(
for path in results {
if let Some(path) = normalize_path(path) {
let request = Request::parse(Value::new(format!("./{}", path).into()));
resolved_results.push(resolve_internal_boxed(package_path, request, options).await?);
resolved_results.push(resolve_internal(package_path, request, options));
}
}

Expand Down
18 changes: 10 additions & 8 deletions crates/turbopack-core/src/resolve/options.rs
Original file line number Diff line number Diff line change
Expand Up @@ -345,26 +345,28 @@ fn import_mapping_to_result_boxed(
Box::pin(async move { import_mapping_to_result(mapping, lookup_path, request).await })
}

#[turbo_tasks::value_impl]
impl ImportMap {
// Not a turbo-tasks function: the map lookup should be cheaper than the cache
// lookup
#[turbo_tasks::function]
pub async fn lookup(
&self,
self: Vc<Self>,
lookup_path: Vc<FileSystemPath>,
request: Vc<Request>,
) -> Result<ImportMapResult> {
) -> Result<Vc<ImportMapResult>> {
let this = self.await?;
// TODO lookup pattern
if let Some(request_string) = request.await?.request() {
if let Some(result) = self.map.lookup(&request_string).next() {
return import_mapping_to_result(
if let Some(result) = this.map.lookup(&request_string).next() {
return Ok(import_mapping_to_result(
result.try_join_into_self().await?.into_owned(),
lookup_path,
request,
)
.await;
.await?
.into());
}
}
Ok(ImportMapResult::NoEntry)
Ok(ImportMapResult::NoEntry.into())
}
}

Expand Down
Loading

0 comments on commit c39c93d

Please sign in to comment.