Skip to content

Commit 7dff290

Browse files
committed
speedup attempt
1 parent 7744db5 commit 7dff290

File tree

2 files changed

+57
-36
lines changed

2 files changed

+57
-36
lines changed

src/dfx/src/lib/builders/motoko.rs

Lines changed: 30 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ use crate::lib::canister_info::CanisterInfo;
66
use crate::lib::environment::Environment;
77
use crate::lib::error::{BuildError, DfxError, DfxResult};
88
use crate::lib::metadata::names::{CANDID_ARGS, CANDID_SERVICE};
9-
use crate::lib::models::canister::CanisterPool;
9+
use crate::lib::models::canister::{CanisterPool, ImportsTracker, MotokoImport};
1010
use crate::lib::package_arguments::{self, PackageArguments};
1111
use crate::util::assets::management_idl;
1212
use crate::lib::builders::bail;
@@ -17,9 +17,8 @@ use dfx_core::config::model::dfinity::{MetadataVisibility, Profile};
1717
use dfx_core::fs::metadata;
1818
use fn_error_context::context;
1919
use slog::{info, o, trace, warn, Logger};
20-
use std::collections::{BTreeMap, BTreeSet};
20+
use std::collections::BTreeMap;
2121
use std::convert::TryFrom;
22-
use std::fmt::Debug;
2322
use std::path::{Path, PathBuf};
2423
use std::process::Output;
2524
use std::sync::Arc;
@@ -43,20 +42,19 @@ impl MotokoBuilder {
4342
}
4443
}
4544

45+
// TODO: Rename this function.
4646
#[context("Failed to find imports for canister at '{}'.", info.get_main_path().display())]
47-
fn get_imports(cache: &dyn Cache, info: &MotokoCanisterInfo) -> DfxResult<BTreeSet<MotokoImport>> {
48-
#[context("Failed recursive dependency detection at {}.", file.display())]
49-
fn get_imports_recursive(
47+
fn get_imports(cache: &dyn Cache, info: &MotokoCanisterInfo, imports: &mut ImportsTracker) -> DfxResult<()> {
48+
#[context("Failed recursive dependency detection at {}.", file.display())] // FIXME
49+
fn get_imports_recursive (
5050
cache: &dyn Cache,
5151
file: &Path,
52-
result: &mut BTreeSet<MotokoImport>,
52+
imports: &mut ImportsTracker,
5353
) -> DfxResult {
54-
if result.contains(&MotokoImport::Relative(file.to_path_buf())) {
54+
if imports.nodes.contains_key(&MotokoImport::Relative(file.to_path_buf())) {
5555
return Ok(());
5656
}
5757

58-
result.insert(MotokoImport::Relative(file.to_path_buf()));
59-
6058
let mut command = cache.get_binary_command("moc")?;
6159
let command = command.arg("--print-deps").arg(file);
6260
let output = command
@@ -68,21 +66,25 @@ fn get_imports(cache: &dyn Cache, info: &MotokoCanisterInfo) -> DfxResult<BTreeS
6866
let import = MotokoImport::try_from(line).context("Failed to create MotokoImport.")?;
6967
match import {
7068
MotokoImport::Relative(path) => {
71-
get_imports_recursive(cache, path.as_path(), result)?;
69+
get_imports_recursive(cache, path.as_path(), imports)?;
7270
}
7371
_ => {
74-
result.insert(import);
72+
let parent = MotokoImport::Relative(file.to_path_buf());
73+
// imports.insert(parent);
74+
75+
let parent_node = imports.graph.add_node(parent);
76+
let child_node = imports.graph.add_node(import);
77+
imports.graph.add_edge(parent_node, child_node, ());
7578
}
7679
}
7780
}
7881

7982
Ok(())
8083
}
8184

82-
let mut result = BTreeSet::new();
83-
get_imports_recursive(cache, info.get_main_path(), &mut result)?;
85+
get_imports_recursive(cache, info.get_main_path(), imports)?;
8486

85-
Ok(result)
87+
Ok(())
8688
}
8789

8890
impl CanisterBuilder for MotokoBuilder {
@@ -93,13 +95,13 @@ impl CanisterBuilder for MotokoBuilder {
9395
info: &CanisterInfo,
9496
) -> DfxResult<Vec<CanisterId>> {
9597
let motoko_info = info.as_info::<MotokoCanisterInfo>()?;
96-
let imports = get_imports(self.cache.as_ref(), &motoko_info)?; // TODO: slow operation
98+
get_imports(self.cache.as_ref(), &motoko_info, &mut *pool.imports.borrow_mut())?; // TODO: slow operation
9799

98-
Ok(imports
100+
Ok(pool.imports.borrow_mut().nodes
99101
.iter()
100102
.filter_map(|import| {
101-
if let MotokoImport::Canister(name) = import {
102-
pool.get_first_canister_with_name(name)
103+
if let MotokoImport::Canister(name) = import.0 {
104+
pool.get_first_canister_with_name(name.as_str())
103105
} else {
104106
None
105107
}
@@ -144,10 +146,10 @@ impl CanisterBuilder for MotokoBuilder {
144146
std::fs::create_dir_all(idl_dir_path)
145147
.with_context(|| format!("Failed to create {}.", idl_dir_path.to_string_lossy()))?;
146148

147-
let imports = get_imports(cache.as_ref(), &motoko_info)?; // TODO: repeated slow operation
149+
get_imports(cache.as_ref(), &motoko_info, &mut *pool.imports.borrow_mut())?; // TODO: repeated slow operation
148150

149151
// If the management canister is being imported, emit the candid file.
150-
if imports.contains(&MotokoImport::Ic("aaaaa-aa".to_string()))
152+
if pool.imports.borrow().nodes.contains_key(&MotokoImport::Ic("aaaaa-aa".to_string()))
151153
{
152154
let management_idl_path = idl_dir_path.join("aaaaa-aa.did");
153155
dfx_core::fs::write(management_idl_path, management_idl()?)?;
@@ -171,12 +173,13 @@ impl CanisterBuilder for MotokoBuilder {
171173
// Check that one of the dependencies is newer than the target:
172174
if let Ok(wasm_file_metadata) = metadata(output_wasm_path) {
173175
let wasm_file_time = wasm_file_metadata.modified()?;
174-
let mut import_iter = imports.iter();
176+
let imports = pool.imports.borrow();
177+
let mut import_iter = imports.nodes.iter();
175178
loop {
176179
if let Some(import) = import_iter.next() {
177-
let imported_file = match import {
180+
let imported_file = match import.0 {
178181
MotokoImport::Canister(canister_name) => {
179-
if let Some(canister) = pool.get_first_canister_with_name(canister_name) {
182+
if let Some(canister) = pool.get_first_canister_with_name(canister_name.as_str()) {
180183
let main_file = canister.get_info().get_main_file();
181184
if let Some(main_file) = main_file {
182185
Some(main_file.to_owned())
@@ -188,7 +191,7 @@ impl CanisterBuilder for MotokoBuilder {
188191
}
189192
}
190193
MotokoImport::Ic(canister_id) => {
191-
if let Some(canister_name) = rev_id_map.get(canister_id) {
194+
if let Some(canister_name) = rev_id_map.get(canister_id.as_str()) {
192195
if let Some(canister) = pool.get_first_canister_with_name(canister_name) {
193196
if let Some(main_file) = canister.get_info().get_main_file() {
194197
Some(main_file.to_owned())
@@ -210,7 +213,7 @@ impl CanisterBuilder for MotokoBuilder {
210213
);
211214
expanded.join(&path[i+1..])
212215
} else {
213-
Path::new(path).to_owned()
216+
Path::new(path.as_str()).to_owned()
214217
};
215218
let path2 = pre_path.to_string_lossy() + ".mo"; // TODO: Is `lossy` OK?
216219
let path2 = path2.to_string();
@@ -227,7 +230,7 @@ impl CanisterBuilder for MotokoBuilder {
227230
}
228231
}
229232
MotokoImport::Relative(path) => {
230-
Some(Path::new(path).to_owned())
233+
Some(Path::new(&path).to_owned())
231234
}
232235
};
233236
if let Some(imported_file) = imported_file {
@@ -364,14 +367,6 @@ fn motoko_compile(logger: &Logger, cache: &dyn Cache, params: &MotokoParams<'_>)
364367
Ok(())
365368
}
366369

367-
#[derive(Debug, PartialOrd, Ord, PartialEq, Eq)]
368-
enum MotokoImport {
369-
Canister(String),
370-
Ic(String),
371-
Lib(String),
372-
Relative(PathBuf),
373-
}
374-
375370
impl TryFrom<&str> for MotokoImport {
376371
type Error = DfxError;
377372

src/dfx/src/lib/models/canister.rs

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ use std::collections::{BTreeMap, HashMap, HashSet};
2828
use std::convert::TryFrom;
2929
use std::ffi::OsStr;
3030
use std::io::Read;
31-
use std::path::Path;
31+
use std::path::{Path, PathBuf};
3232
use std::process::{Command, Stdio};
3333
use std::sync::Arc;
3434

@@ -435,10 +435,35 @@ fn check_valid_subtype(compiled_idl_path: &Path, specified_idl_path: &Path) -> D
435435
Ok(())
436436
}
437437

438+
/// TODO: Motoko-specific code not here
439+
#[derive(Debug, PartialOrd, Ord, PartialEq, Eq, Hash)]
440+
pub enum MotokoImport {
441+
Canister(String),
442+
Ic(String),
443+
Lib(String),
444+
Relative(PathBuf),
445+
}
446+
447+
/// The graph of Motoko imports (TODO: Motoko-specific code not here)
448+
pub struct ImportsTracker {
449+
pub nodes: HashMap<MotokoImport, ()>,
450+
pub graph: DiGraph<MotokoImport, ()>,
451+
}
452+
453+
impl ImportsTracker {
454+
pub fn new() -> Self {
455+
Self {
456+
nodes: HashMap::new(),
457+
graph: DiGraph::new(),
458+
}
459+
}
460+
}
461+
438462
/// A canister pool is a list of canisters.
439463
pub struct CanisterPool {
440464
canisters: Vec<Arc<Canister>>,
441465
logger: Logger,
466+
pub imports: RefCell<ImportsTracker>, // TODO: `pub` is a bad habit.
442467
}
443468

444469
struct PoolConstructHelper<'a> {
@@ -493,6 +518,7 @@ impl CanisterPool {
493518
Ok(CanisterPool {
494519
canisters: canisters_map,
495520
logger,
521+
imports: RefCell::new(ImportsTracker::new()),
496522
})
497523
}
498524

0 commit comments

Comments
 (0)