Skip to content

Commit

Permalink
Memory usage reduction (#6249)
Browse files Browse the repository at this point in the history
## Description

#6044 increased the memory usage of the compiler by an estimated 400%.
This PR alleviates some of that increased footprint.

`is_external` in the `Module` struct needs to be updated to allow for
compilation of dependencies (a module is not external while it is being
compiled, but is external when one of its dependents is being compiled).
#6044 introduced an extra level of cloning, which in this PR is changed
by making `Module` mutable and destructively updating `is_external`.

## Checklist

- [x] I have linked to any relevant issues.
- [x] I have commented my code, particularly in hard-to-understand
areas.
- [x] I have updated the documentation where relevant (API docs, the
reference, and the Sway book).
- [ ] If my change requires substantial documentation changes, I have
[requested support from the DevRel
team](https://github.com/FuelLabs/devrel-requests/issues/new/choose)
- [x] I have added tests that prove my fix is effective or that my
feature works.
- [x] I have added (or requested a maintainer to add) the necessary
`Breaking*` or `New Feature` labels where relevant.
- [x] I have done my best to ensure that my PR adheres to [the Fuel Labs
Code Review
Standards](https://github.com/FuelLabs/rfcs/blob/master/text/code-standards/external-contributors.md).
- [x] I have requested a review from the relevant team or maintainers.
  • Loading branch information
jjcnn authored and esdrubal committed Aug 13, 2024
1 parent 3cccf6b commit 0dd74ca
Show file tree
Hide file tree
Showing 8 changed files with 38 additions and 43 deletions.
14 changes: 7 additions & 7 deletions forc-pkg/src/pkg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1759,7 +1759,7 @@ pub fn compile(
pkg: &PackageDescriptor,
profile: &BuildProfile,
engines: &Engines,
namespace: namespace::Root,
namespace: &mut namespace::Root,
source_map: &mut SourceMap,
) -> Result<CompiledPackage> {
let mut metrics = PerformanceData::default();
Expand Down Expand Up @@ -2373,7 +2373,7 @@ pub fn build(

// `ContractIdConst` is a None here since we do not yet have a
// contract ID value at this point.
let dep_namespace = match dependency_namespace(
let mut dep_namespace = match dependency_namespace(
&lib_namespace_map,
&compiled_contract_deps,
plan.graph(),
Expand All @@ -2390,7 +2390,7 @@ pub fn build(
&descriptor,
&profile,
&engines,
dep_namespace,
&mut dep_namespace,
&mut source_map,
)?;

Expand Down Expand Up @@ -2437,7 +2437,7 @@ pub fn build(
};

// Note that the contract ID value here is only Some if tests are enabled.
let dep_namespace = match dependency_namespace(
let mut dep_namespace = match dependency_namespace(
&lib_namespace_map,
&compiled_contract_deps,
plan.graph(),
Expand All @@ -2463,7 +2463,7 @@ pub fn build(
&descriptor,
&profile,
&engines,
dep_namespace,
&mut dep_namespace,
&mut source_map,
)?;

Expand Down Expand Up @@ -2672,7 +2672,7 @@ pub fn check(
let contract_id_value =
(idx == plan.compilation_order.len() - 1).then(|| DUMMY_CONTRACT_ID.to_string());

let dep_namespace = dependency_namespace(
let mut dep_namespace = dependency_namespace(
&lib_namespace_map,
&compiled_contract_deps,
&plan.graph,
Expand Down Expand Up @@ -2703,7 +2703,7 @@ pub fn check(
&handler,
engines,
input,
dep_namespace,
&mut dep_namespace,
Some(&build_config),
&pkg.name,
retrigger_compilation.clone(),
Expand Down
4 changes: 2 additions & 2 deletions sway-core/src/ir_generation/const_eval.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1316,7 +1316,7 @@ mod tests {
},
);
let mut md_mgr = MetadataManager::default();
let core_lib = namespace::Root::from(namespace::Module {
let mut core_lib = namespace::Root::from(namespace::Module {
name: Some(sway_types::Ident::new_no_span(
"assert_is_constant_test".to_string(),
)),
Expand All @@ -1327,7 +1327,7 @@ mod tests {
&handler,
&engines,
std::sync::Arc::from(format!("library; {prefix} fn f() -> u64 {{ {expr}; 0 }}")),
core_lib,
&mut core_lib,
None,
"test",
None,
Expand Down
8 changes: 4 additions & 4 deletions sway-core/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -520,7 +520,7 @@ pub fn parsed_to_ast(
handler: &Handler,
engines: &Engines,
parse_program: &mut parsed::ParseProgram,
initial_namespace: namespace::Root,
initial_namespace: &mut namespace::Root,
build_config: Option<&BuildConfig>,
package_name: &str,
retrigger_compilation: Option<Arc<AtomicBool>>,
Expand Down Expand Up @@ -691,7 +691,7 @@ pub fn compile_to_ast(
handler: &Handler,
engines: &Engines,
input: Arc<str>,
initial_namespace: namespace::Root,
initial_namespace: &mut namespace::Root,
build_config: Option<&BuildConfig>,
package_name: &str,
retrigger_compilation: Option<Arc<AtomicBool>>,
Expand Down Expand Up @@ -783,7 +783,7 @@ pub fn compile_to_asm(
handler: &Handler,
engines: &Engines,
input: Arc<str>,
initial_namespace: namespace::Root,
initial_namespace: &mut namespace::Root,
build_config: &BuildConfig,
package_name: &str,
) -> Result<CompiledAsm, ErrorEmitted> {
Expand Down Expand Up @@ -944,7 +944,7 @@ pub fn compile_to_bytecode(
handler: &Handler,
engines: &Engines,
input: Arc<str>,
initial_namespace: namespace::Root,
initial_namespace: &mut namespace::Root,
build_config: &BuildConfig,
source_map: &mut SourceMap,
package_name: &str,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2663,8 +2663,8 @@ mod tests {
type_annotation: TypeId,
experimental: ExperimentalFlags,
) -> Result<ty::TyExpression, ErrorEmitted> {
let root_module = namespace::Root::from(namespace::Module::default());
let mut namespace = Namespace::init_root(root_module);
let mut root_module = namespace::Root::from(namespace::Module::default());
let mut namespace = Namespace::init_root(&mut root_module);
let ctx = TypeCheckContext::from_namespace(&mut namespace, engines, experimental)
.with_type_annotation(type_annotation);
ty::TyExpression::type_check(handler, ctx, expr)
Expand Down
4 changes: 2 additions & 2 deletions sway-core/src/semantic_analysis/namespace/contract_helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -95,8 +95,8 @@ fn default_with_contract_id_inner(
content: AstNodeContent::Declaration(Declaration::ConstantDeclaration(const_decl_id)),
span: const_item_span.clone(),
};
let root = Root::from(Module::default());
let mut ns = Namespace::init_root(root);
let mut root = Root::from(Module::default());
let mut ns = Namespace::init_root(&mut root);
// This is pretty hacky but that's okay because of this code is being removed pretty soon
ns.root.module.name = ns_name;
ns.root.module.visibility = Visibility::Public;
Expand Down
7 changes: 7 additions & 0 deletions sway-core/src/semantic_analysis/namespace/module.rs
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,13 @@ impl Module {
&self.submodules
}

/// Mutable access to this module's submodules.
pub fn submodules_mut(
&mut self,
) -> &mut im::HashMap<ModuleName, Module, BuildHasherDefault<FxHasher>> {
&mut self.submodules
}

/// Insert a submodule into this `Module`.
pub fn insert_submodule(&mut self, name: String, submodule: Module) {
self.submodules.insert(name, submodule);
Expand Down
36 changes: 12 additions & 24 deletions sway-core/src/semantic_analysis/namespace/namespace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,9 @@ use super::{
root::{ResolvedDeclaration, Root},
submodule_namespace::SubmoduleNamespace,
trait_map::ResolvedTraitImplItem,
ModuleName, ModulePath, ModulePathBuf,
ModulePath, ModulePathBuf,
};

use rustc_hash::FxHasher;
use std::hash::BuildHasherDefault;
use sway_error::handler::{ErrorEmitted, Handler};
use sway_types::span::Span;

Expand Down Expand Up @@ -58,7 +56,7 @@ impl Namespace {

/// Initialise the namespace at its root from the given initial namespace.
/// If the root module contains submodules these are now considered external.
pub fn init_root(root: Root) -> Self {
pub fn init_root(root: &mut Root) -> Self {
assert!(
!root.module.is_external,
"The root module must not be external during compilation"
Expand All @@ -69,32 +67,22 @@ impl Namespace {
//
// Every submodule that has been added before calling init_root is now considered
// external, which we have to enforce at this point.
fn clone_with_submodules_external(module: &Module) -> Module {
let mut new_submods =
im::HashMap::<ModuleName, Module, BuildHasherDefault<FxHasher>>::default();
for (name, submod) in module.submodules.iter() {
let new_submod = clone_with_submodules_external(submod);
new_submods.insert(name.clone(), new_submod);
}
Module {
submodules: new_submods,
lexical_scopes: module.lexical_scopes.clone(),
current_lexical_scope_id: module.current_lexical_scope_id,
name: module.name.clone(),
visibility: module.visibility,
span: module.span.clone(),
is_external: true,
mod_path: module.mod_path.clone(),
fn set_submodules_external(module: &mut Module) {
for (_, submod) in module.submodules_mut().iter_mut() {
if !submod.is_external {
submod.is_external = true;
set_submodules_external(submod);
}
}
}

let mut init = clone_with_submodules_external(&root.module);
set_submodules_external(&mut root.module);
// The init module itself is not external
init.is_external = false;
root.module.is_external = false;

Self {
init: init.clone(),
root: Root { module: init },
init: root.module.clone(),
root: root.clone(),
mod_path,
}
}
Expand Down
4 changes: 2 additions & 2 deletions test/src/ir_generation/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -238,12 +238,12 @@ pub(super) async fn run(

let sway_str = String::from_utf8_lossy(&sway_str);
let handler = Handler::default();
let initial_namespace = namespace::Root::from(core_lib.clone());
let mut initial_namespace = namespace::Root::from(core_lib.clone());
let compile_res = compile_to_ast(
&handler,
&engines,
Arc::from(sway_str),
initial_namespace,
&mut initial_namespace,
Some(&bld_cfg),
PACKAGE_NAME,
None,
Expand Down

0 comments on commit 0dd74ca

Please sign in to comment.