Skip to content

Commit

Permalink
New alg for tree shaking that uses linkage tables for determining which
Browse files Browse the repository at this point in the history
packages to keep for deps.
  • Loading branch information
stefan-mysten committed Feb 27, 2025
1 parent e919e84 commit 2a77055
Show file tree
Hide file tree
Showing 11 changed files with 271 additions and 100 deletions.
2 changes: 2 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

43 changes: 16 additions & 27 deletions crates/sui-move-build/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -619,15 +619,12 @@ impl CompiledPackage {
self.dependency_ids.published.values().cloned().collect()
}

/// Tree-shake the package's dependencies to remove any that are not referenced in source code.
///
/// This algorithm uses the set of root modules as the starting point to retrieve the
/// list of used packages that are immediate dependencies of these modules. Essentially, it
/// will remove any package that has no immediate module dependency to it.
///
/// Then, it will recursively find all the transitive dependencies of the packages in the list
/// above and add them to the list of packages that need to be kept as dependencies.
pub fn tree_shake(&mut self, with_unpublished_deps: bool) -> Result<(), anyhow::Error> {
/// Find the map of packages that are immediate dependencies of the root modules, joined with
/// the set of bytecode dependencies.
pub fn find_immediate_deps_pkgs_to_keep(
&self,
with_unpublished_deps: bool,
) -> Result<BTreeMap<Symbol, ObjectID>, anyhow::Error> {
// Start from the root modules (or all modules if with_unpublished_deps is true as we
// need to include modules with 0x0 address)
let root_modules: Vec<_> = if with_unpublished_deps {
Expand All @@ -644,15 +641,14 @@ impl CompiledPackage {
};

// Find the immediate dependencies for each root module and store the package name
// in the used_immediate_packages set. This basically prunes the packages that are not used
// in the pkgs_to_keep set. This basically prunes the packages that are not used
// based on the modules information.
let mut pkgs_to_keep: BTreeSet<Symbol> = BTreeSet::new();
let module_to_pkg_name: BTreeMap<_, _> = self
.package
.all_modules()
.map(|m| (m.unit.module.self_id(), m.unit.package_name))
.collect();
let mut used_immediate_packages: BTreeSet<Symbol> = BTreeSet::new();

for module in &root_modules {
let immediate_deps = module.module.immediate_dependencies();
Expand All @@ -661,31 +657,24 @@ impl CompiledPackage {
let Some(pkg_name) = pkg_name else {
bail!("Expected a package name but it's None")
};
used_immediate_packages.insert(*pkg_name);
pkgs_to_keep.insert(*pkg_name);
}
}
}

// Next, for each package from used_immediate_packages set, we need to find all the
// transitive dependencies. Those trans dependencies need to be included in the final list
// of package dependencies (note that the pkg itself will be added by the transitive deps
// function)
used_immediate_packages.iter().for_each(|pkg| {
self.dependency_graph
.add_transitive_dependencies(pkg, &mut pkgs_to_keep)
});

// If a package depends on another published package that has only bytecode without source
// code available, we need to include also that package as dep.
pkgs_to_keep.extend(self.bytecode_deps.iter().map(|(name, _)| *name));

// Finally, filter out packages that are not in the union above from the
// dependency_ids.published field and return the package ids.
self.dependency_ids
// Finally, filter out packages that are published and exist in the manifest at the
// compilation time but are not referenced in the source code.
Ok(self
.dependency_ids
.clone()
.published
.retain(|pkg_name, _| pkgs_to_keep.contains(pkg_name));

Ok(())
.into_iter()
.filter(|(pkg_name, _)| pkgs_to_keep.contains(pkg_name))
.collect())
}
}

Expand Down
25 changes: 2 additions & 23 deletions crates/sui-move/src/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,8 @@ use crate::manage_package::resolve_lock_file_path;
use clap::Parser;
use move_cli::base;
use move_package::BuildConfig as MoveBuildConfig;
use serde_json::json;
use std::{fs, path::Path};
use sui_move_build::{check_invalid_dependencies, check_unpublished_dependencies, BuildConfig};
use sui_move_build::BuildConfig;

const LAYOUTS_DIR: &str = "layouts";
const STRUCT_LAYOUTS_FILENAME: &str = "struct_layouts.yaml";
Expand Down Expand Up @@ -52,8 +51,6 @@ impl Build {
Self::execute_internal(
&rerooted_path,
build_config,
self.with_unpublished_dependencies,
self.dump_bytecode_as_base64,
self.generate_struct_layouts,
self.chain_id.clone(),
)
Expand All @@ -62,34 +59,16 @@ impl Build {
pub fn execute_internal(
rerooted_path: &Path,
config: MoveBuildConfig,
with_unpublished_deps: bool,
dump_bytecode_as_base64: bool,
generate_struct_layouts: bool,
chain_id: Option<String>,
) -> anyhow::Result<()> {
let mut pkg = BuildConfig {
let pkg = BuildConfig {
config,
run_bytecode_verifier: true,
print_diags_to_stderr: true,
chain_id,
}
.build(rerooted_path)?;
if dump_bytecode_as_base64 {
check_invalid_dependencies(&pkg.dependency_ids.invalid)?;
if !with_unpublished_deps {
check_unpublished_dependencies(&pkg.dependency_ids.unpublished)?;
}

pkg.tree_shake(with_unpublished_deps)?;
println!(
"{}",
json!({
"modules": pkg.get_package_base64(with_unpublished_deps),
"dependencies": pkg.get_dependency_storage_package_ids(),
"digest": pkg.get_package_digest(with_unpublished_deps),
})
)
}

if generate_struct_layouts {
let layout_str = serde_yaml::to_string(&pkg.generate_struct_layouts()).unwrap();
Expand Down
2 changes: 2 additions & 0 deletions crates/sui/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,8 @@ move-vm-profiler.workspace = true
move-vm-config.workspace = true
move-ir-types.workspace = true
move-command-line-common.workspace = true
move-cli.workspace = true
move-symbol-pool.workspace = true

[target.'cfg(not(target_env = "msvc"))'.dependencies]
jemalloc-ctl.workspace = true
Expand Down
Loading

0 comments on commit 2a77055

Please sign in to comment.