Skip to content

Commit

Permalink
Performance Improvements (5) (#6076)
Browse files Browse the repository at this point in the history
### Description

The `AsyncModule` code generation in every ESM flags it as "needs
availability info" which causes code generation to be duplicated for
each entrypoints.

This improves it a little bit by checking is the module is an async
module before. It also fixes a bug where availability is not correct
reduced and only passes the availability root to async module
computation instead of the full availability info.

### Testing Instructions

<!--
  Give a quick description of steps to test your changes.
-->


Closes WEB-1697
  • Loading branch information
sokra authored Oct 6, 2023
1 parent 59ac153 commit 624521f
Show file tree
Hide file tree
Showing 9 changed files with 230 additions and 61 deletions.
2 changes: 1 addition & 1 deletion crates/turbo-tasks/src/manager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -410,7 +410,7 @@ impl<B: Backend + 'static> TurboTasks<B> {
/// Calls a native function with arguments. Resolves arguments when needed
/// with a wrapper [Task].
pub fn dynamic_call(&self, func: FunctionId, inputs: Vec<ConcreteTaskInput>) -> RawVc {
if inputs.iter().all(|i| i.is_resolved() && !i.is_nothing()) {
if inputs.iter().all(|i| i.is_resolved()) {
self.native_call(func, inputs)
} else {
RawVc::TaskOutput(self.backend.get_or_create_persistent_task(
Expand Down
110 changes: 107 additions & 3 deletions crates/turbopack-core/src/chunk/availability_info.rs
Original file line number Diff line number Diff line change
@@ -1,19 +1,84 @@
use std::ops::{BitOr, BitOrAssign};

use turbo_tasks::Vc;

use super::available_modules::AvailableAssets;
use crate::module::Module;

/// This specifies which information is needed from an AvailabilityInfo
#[turbo_tasks::value(shared)]
#[derive(Copy, Clone, Debug)]
pub struct AvailabilityInfoNeeds {
pub current_availability_root: bool,
pub available_modules: bool,
}

impl AvailabilityInfoNeeds {
pub fn all() -> Self {
Self {
current_availability_root: true,
available_modules: true,
}
}

pub fn none() -> Self {
Self {
current_availability_root: false,
available_modules: false,
}
}

pub fn is_complete(self) -> bool {
let Self {
current_availability_root,
available_modules,
} = self;
current_availability_root && available_modules
}
}

impl BitOr for AvailabilityInfoNeeds {
type Output = AvailabilityInfoNeeds;

fn bitor(mut self, rhs: Self) -> Self::Output {
let Self {
available_modules,
current_availability_root,
} = rhs;
self.current_availability_root |= current_availability_root;
self.available_modules |= available_modules;
self
}
}

impl BitOrAssign for AvailabilityInfoNeeds {
fn bitor_assign(&mut self, rhs: Self) {
*self = *self | rhs;
}
}

#[turbo_tasks::value(serialization = "auto_for_input")]
#[derive(PartialOrd, Ord, Hash, Clone, Copy, Debug)]
pub enum AvailabilityInfo {
/// Available modules are not tracked or the information is not available
/// due to specified needs.
Untracked,
/// There are no modules available (or the information is not available due
/// to specified needs), but it's tracked for nested chunk groups
/// and the current chunk group root is defined.
Root {
current_availability_root: Vc<Box<dyn Module>>,
},
Inner {
/// There are modules already available and the current chunk group root is
/// defined.
Complete {
available_modules: Vc<AvailableAssets>,
current_availability_root: Vc<Box<dyn Module>>,
},
/// Only partial information is available, about the available modules.
OnlyAvailableModules {
available_modules: Vc<AvailableAssets>,
},
}

impl AvailabilityInfo {
Expand All @@ -23,20 +88,59 @@ impl AvailabilityInfo {
Self::Root {
current_availability_root,
} => Some(*current_availability_root),
Self::Inner {
Self::Complete {
current_availability_root,
..
} => Some(*current_availability_root),
Self::OnlyAvailableModules { .. } => None,
}
}

pub fn available_modules(&self) -> Option<Vc<AvailableAssets>> {
match self {
Self::Untracked => None,
Self::Root { .. } => None,
Self::Inner {
Self::Complete {
available_modules, ..
} => Some(*available_modules),
Self::OnlyAvailableModules {
available_modules, ..
} => Some(*available_modules),
}
}

/// Returns AvailabilityInfo, but only with the information that is needed
pub fn reduce_to_needs(self, needs: AvailabilityInfoNeeds) -> Self {
let AvailabilityInfoNeeds {
available_modules,
current_availability_root,
} = needs;
match (current_availability_root, available_modules, self) {
(false, false, _) => Self::Untracked,
(true, true, _) => self,
(
true,
false,
Self::Root {
current_availability_root,
}
| Self::Complete {
current_availability_root,
..
},
) => Self::Root {
current_availability_root,
},
(true, false, _) => Self::Untracked,
(
false,
true,
Self::Complete {
available_modules, ..
}
| Self::OnlyAvailableModules { available_modules },
) => Self::OnlyAvailableModules { available_modules },
(false, true, _) => Self::Untracked,
}
}
}
8 changes: 4 additions & 4 deletions crates/turbopack-ecmascript/src/chunk/esm_scope.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,9 @@ use std::collections::HashMap;

use anyhow::{Context, Result};
use petgraph::{algo::tarjan_scc, prelude::DiGraphMap};
use turbo_tasks::{TryFlatJoinIterExt, Value, Vc};
use turbo_tasks::{TryFlatJoinIterExt, Vc};
use turbopack_core::{
chunk::{availability_info::AvailabilityInfo, available_modules::chunkable_modules_set},
chunk::available_modules::chunkable_modules_set,
module::{Module, ModulesSet},
};

Expand Down Expand Up @@ -38,8 +38,8 @@ pub(crate) struct EsmScopeSccs(Vec<Vc<EsmScopeScc>>);
impl EsmScope {
/// Create a new [EsmScope] from the availability root given.
#[turbo_tasks::function]
pub(crate) async fn new(availability_info: Value<AvailabilityInfo>) -> Result<Vc<Self>> {
let assets = if let Some(root) = availability_info.current_availability_root() {
pub(crate) async fn new(availability_root: Option<Vc<Box<dyn Module>>>) -> Result<Vc<Self>> {
let assets = if let Some(root) = availability_root {
chunkable_modules_set(root)
} else {
ModulesSet::empty()
Expand Down
9 changes: 6 additions & 3 deletions crates/turbopack-ecmascript/src/chunk/item.rs
Original file line number Diff line number Diff line change
Expand Up @@ -271,17 +271,20 @@ impl FromChunkableModule for Box<dyn EcmascriptChunkItem> {
AvailabilityInfo::Untracked => AvailabilityInfo::Untracked,
AvailabilityInfo::Root {
current_availability_root,
} => AvailabilityInfo::Inner {
} => AvailabilityInfo::Complete {
available_modules: AvailableAssets::new(vec![current_availability_root]),
current_availability_root: Vc::upcast(module),
},
AvailabilityInfo::Inner {
AvailabilityInfo::Complete {
available_modules,
current_availability_root,
} => AvailabilityInfo::Inner {
} => AvailabilityInfo::Complete {
available_modules: available_modules.with_roots(vec![current_availability_root]),
current_availability_root: Vc::upcast(module),
},
AvailabilityInfo::OnlyAvailableModules { .. } => {
panic!("OnlyAvailableModules should not appear here")
}
};

let manifest_asset =
Expand Down
11 changes: 10 additions & 1 deletion crates/turbopack-ecmascript/src/code_gen.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use serde::{Deserialize, Serialize};
use swc_core::ecma::visit::{AstParentKind, VisitMut};
use turbo_tasks::{debug::ValueDebugFormat, trace::TraceRawVcs, Value, Vc};
use turbopack_core::chunk::availability_info::AvailabilityInfo;
use turbopack_core::chunk::availability_info::{AvailabilityInfo, AvailabilityInfoNeeds};

use crate::chunk::EcmascriptChunkingContext;

Expand Down Expand Up @@ -34,6 +34,15 @@ pub trait CodeGenerateable {

#[turbo_tasks::value_trait]
pub trait CodeGenerateableWithAvailabilityInfo {
/// Returns the pieces of AvailabilityInfo that are needed for
/// `code_generation`.
fn get_availability_info_needs(
self: Vc<Self>,
_is_async_module: bool,
) -> Vc<AvailabilityInfoNeeds> {
AvailabilityInfoNeeds::all().cell()
}

fn code_generation(
self: Vc<Self>,
chunking_context: Vc<Box<dyn EcmascriptChunkingContext>>,
Expand Down
24 changes: 15 additions & 9 deletions crates/turbopack-ecmascript/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -353,11 +353,6 @@ impl EcmascriptModuleAsset {
availability_info: Value<AvailabilityInfo>,
) -> Result<Vc<EcmascriptModuleContent>> {
let this = self.await?;
let availability_info = if *self.analyze().needs_availability_info().await? {
availability_info
} else {
Value::new(AvailabilityInfo::Untracked)
};

let parsed = parse(this.source, Value::new(this.ty), this.transforms);

Expand Down Expand Up @@ -515,13 +510,24 @@ impl EcmascriptChunkItem for ModuleChunkItem {
availability_info: Value<AvailabilityInfo>,
) -> Result<Vc<EcmascriptChunkItemContent>> {
let this = self.await?;
let content = this
.module
.module_content(this.chunking_context, availability_info);
let async_module_options = this
.module
.get_async_module()
.module_options(availability_info);
.module_options(availability_info.current_availability_root());
let is_async_module = async_module_options.await?.is_some();
let availability_info_needs = *this
.module
.analyze()
.get_availability_info_needs(is_async_module)
.await?;
// We reduce the availability info to the needs of the chunk item to improve
// caching of the methods that are called with availability info. e. g.
// module_content() can be cached for different availability info when it
// doesn't really need that info.
let availability_info = availability_info.reduce_to_needs(availability_info_needs);
let content = this
.module
.module_content(this.chunking_context, Value::new(availability_info));

Ok(EcmascriptChunkItemContent::new(
content,
Expand Down
Loading

0 comments on commit 624521f

Please sign in to comment.