diff --git a/compiler/load/src/file.rs b/compiler/load/src/file.rs index db0d9ecced..03480a0cf1 100644 --- a/compiler/load/src/file.rs +++ b/compiler/load/src/file.rs @@ -37,7 +37,7 @@ const ROC_FILE_EXTENSION: &str = "roc"; const MODULE_SEPARATOR: char = '.'; #[derive(Debug)] -pub struct LoadedModule<'a> { +pub struct LoadedModule { pub module_id: ModuleId, pub interns: Interns, pub solved: Solved, @@ -45,7 +45,7 @@ pub struct LoadedModule<'a> { pub type_problems: Vec, pub declarations_by_id: MutMap>, pub exposed_vars_by_symbol: Vec<(Symbol, Variable)>, - pub src: &'a str, + pub src: Box, } #[derive(Debug)] @@ -242,70 +242,29 @@ type MsgReceiver<'a> = Receiver>; /// to rebuild the module and can link in the cached one directly.) #[allow(clippy::cognitive_complexity)] pub fn load<'a>( - arena: &'a Bump, stdlib: &StdLib, src_dir: &Path, filename: PathBuf, exposed_types: SubsByModule, -) -> Result, LoadingProblem> { +) -> Result { use self::MaybeShared::*; - let (msg_tx, msg_rx) = bounded(1024); let arc_modules = Arc::new(Mutex::new(ModuleIds::default())); let root_exposed_ident_ids = IdentIds::exposed_builtins(0); let ident_ids_by_module = Arc::new(Mutex::new(root_exposed_ident_ids)); + let arena = Bump::new(); + let (msg_tx, msg_rx) = bounded(1024); // Load the root module synchronously; we can't proceed until we have its id. let root_id = load_filename( - &arena, + arena, filename, - msg_tx.clone(), + &msg_tx, Shared(Arc::clone(&arc_modules), Arc::clone(&ident_ids_by_module)), // TODO FIXME go back to using Unique here, not Shared // Unique(&mut module_ids, &mut root_exposed_ident_ids), )?; - load_deps( - &arena, - root_id, - msg_tx, - msg_rx, - stdlib, - src_dir, - arc_modules, - ident_ids_by_module, - exposed_types, - ) -} - -/// Add a task to the queue, and notify all the listeners. -fn enqueue_task<'a, 'b>( - injector: &Injector>, - listeners: &[Sender], - task: BuildTask<'a, 'b>, -) -> Result<(), LoadingProblem> { - injector.push(task); - - for listener in listeners { - listener - .send(WorkerMsg::TaskAdded) - .map_err(|_| LoadingProblem::MsgChannelDied)?; - } - - Ok(()) -} - -fn load_deps<'a>( - arena: &'a Bump, - root_id: ModuleId, - msg_tx: MsgSender<'a>, - msg_rx: MsgReceiver<'a>, - stdlib: &StdLib, - src_dir: &Path, - arc_modules: Arc>, - ident_ids_by_module: Arc>, - exposed_types: SubsByModule, -) -> Result, LoadingProblem> { // Reserve one CPU for the main thread, and let all the others be eligible // to spawn workers. let num_workers = num_cpus::get() - 1; @@ -317,14 +276,17 @@ fn load_deps<'a>( // into the worker threads, because those workers' stealers need to be // shared between all threads, and this coordination work is much easier // on the main thread. - let mut worker_queues = bumpalo::collections::Vec::with_capacity_in(num_workers, arena); - let mut stealers = bumpalo::collections::Vec::with_capacity_in(num_workers, arena); + let mut worker_queues = bumpalo::collections::Vec::with_capacity_in(num_workers, &arena); + let mut stealers = bumpalo::collections::Vec::with_capacity_in(num_workers, &arena); + + let mut arenas = bumpalo::collections::Vec::with_capacity_in(num_workers, &arena); for _ in 0..num_workers { let worker = Worker::new_lifo(); stealers.push(worker.stealer()); worker_queues.push(worker); + arenas.push(Bump::new()); } // Get a reference to the completed stealers, so we can send that @@ -365,10 +327,11 @@ fn load_deps<'a>( unsolved_modules: MutMap::default(), }; - let mut worker_listeners = bumpalo::collections::Vec::with_capacity_in(num_workers, arena); + let mut worker_listeners = bumpalo::collections::Vec::with_capacity_in(num_workers, &arena); for _ in 0..num_workers { - let arena = Bump::new(); + let msg_tx = msg_tx.clone(); + let mut arena = Bump::new(); let worker = worker_queues.pop().unwrap(); let (worker_msg_tx, worker_msg_rx) = bounded(1024); @@ -381,8 +344,6 @@ fn load_deps<'a>( // Record this thread's handle so the main thread can join it later. thread_scope.spawn(move |_| { - let msg_tx = msg_tx.clone(); - // Keep listening until we receive a Shutdown msg for msg in worker_msg_rx.iter() { match msg { @@ -391,7 +352,7 @@ fn load_deps<'a>( // shut down the thread, so when the main thread // blocks on joining with all the worker threads, // it can finally exit too! - return; + return Ok(arena); } WorkerMsg::TaskAdded => { // Find a task - either from this thread's queue, @@ -399,7 +360,10 @@ fn load_deps<'a>( // queue - and run it. match find_task(&worker, injector, stealers) { Some(task) => { - run_task(task, &arena, src_dir, msg_tx.clone(), stdlib); + arena = match run_task(task, arena, src_dir, &msg_tx, stdlib) { + Ok(arena) => arena, + Err(problem) => return Err(problem), + }; } None => { // No tasks to work on! This might be because @@ -413,9 +377,7 @@ fn load_deps<'a>( } } - // Needed to prevent a borrow checker error about this closure - // outliving its enclosing function. - drop(worker_msg_rx); + Ok(arena) }); } @@ -427,7 +389,6 @@ fn load_deps<'a>( // Grab a reference to these Senders outside the loop, so we can share // it across each iteration of the loop. let worker_listeners = worker_listeners.into_bump_slice(); - let msg_tx = msg_tx.clone(); // The root module will have already queued up messages to process, // and processing those messages will in turn queue up more messages. @@ -455,14 +416,7 @@ fn load_deps<'a>( // This is where most of the main thread's work gets done. // Everything up to this point has been setting up the threading // system which lets this logic work efficiently. - state = update( - state, - msg, - stdlib, - msg_tx.clone(), - &injector, - worker_listeners, - )?; + state = update(state, msg, stdlib, &msg_tx, &injector, worker_listeners)?; } } } @@ -477,7 +431,7 @@ fn update<'a>( mut state: State<'a>, msg: Msg<'a>, stdlib: &StdLib, - msg_tx: MsgSender<'a>, + msg_tx: &MsgSender<'a>, injector: &Injector>, worker_listeners: &'a [Sender], ) -> Result, LoadingProblem> { @@ -673,7 +627,7 @@ fn update<'a>( constraint, var_store, imported_modules, - msg_tx.clone(), + msg_tx, &mut state.exposed_types, stdlib, ), @@ -755,7 +709,7 @@ fn update<'a>( constraint, var_store, imported_modules, - msg_tx.clone(), + msg_tx, &mut state.exposed_types, stdlib, ), @@ -779,7 +733,7 @@ fn finish<'a>( problems: Vec, exposed_vars_by_symbol: Vec<(Symbol, Variable)>, src: &'a str, -) -> LoadedModule<'a> { +) -> LoadedModule { state.type_problems.extend(problems); let module_ids = Arc::try_unwrap(state.arc_modules) @@ -806,10 +760,10 @@ fn finish<'a>( /// Load a module by its module name, rather than by its filename fn load_module<'a>( - arena: &'a Bump, + arena: Bump, src_dir: &Path, module_name: ModuleName, - msg_tx: MsgSender<'a>, + msg_tx: &MsgSender<'a>, module_ids: SharedModules<'a, '_>, ) -> Result { let mut filename = PathBuf::new(); @@ -854,9 +808,9 @@ fn find_task(local: &Worker, global: &Injector, stealers: &[Stealer] } fn parse_src<'a>( - arena: &'a Bump, + arena: Bump, filename: PathBuf, - msg_tx: MsgSender<'a>, + msg_tx: &MsgSender<'a>, module_ids: SharedModules<'_, '_>, src_bytes: &'a [u8], ) -> Result { @@ -911,9 +865,9 @@ fn parse_src<'a>( /// * memory map the filename instead of doing a buffered read /// * assume the contents of the file are valid UTF-8 fn load_filename<'a>( - arena: &'a Bump, + arena: Bump, filename: PathBuf, - msg_tx: MsgSender<'a>, + msg_tx: &MsgSender<'a>, module_ids: SharedModules<'a, '_>, ) -> Result { match fs::read(&filename) { @@ -931,7 +885,7 @@ fn send_header<'a>( imports: &'a [Located>], parse_state: parser::State<'a>, shared_modules: SharedModules<'_, '_>, - msg_tx: MsgSender<'a>, + msg_tx: &MsgSender<'a>, ) -> ModuleId { use MaybeShared::*; @@ -1133,9 +1087,9 @@ impl<'a, 'b> BuildTask<'a, 'b> { module: Module, src: &'a str, constraint: Constraint, - mut var_store: VarStore, + var_store: VarStore, imported_modules: MutSet, - msg_tx: MsgSender, + msg_tx: &MsgSender, exposed_types: &mut SubsByModule, stdlib: &StdLib, ) -> Self { @@ -1395,11 +1349,11 @@ fn ident_from_exposed(entry: &ExposesEntry<'_>) -> Ident { fn run_task<'a, 'b>( task: BuildTask<'a, 'b>, - arena: &'a Bump, + arena: Bump, src_dir: &Path, - msg_tx: MsgSender<'a>, + msg_tx: &MsgSender<'a>, stdlib: &StdLib, -) -> Result<(), LoadingProblem> { +) -> Result { use BuildTask::*; match task { @@ -1409,7 +1363,7 @@ fn run_task<'a, 'b>( } => { let module_id = load_module(arena, src_dir, module_name, msg_tx, module_ids)?; - Ok(()) + Ok(arena) } ParseAndConstrain { header, @@ -1419,7 +1373,7 @@ fn run_task<'a, 'b>( exposed_symbols, } => { //TODO - Ok(()) + Ok(arena) } Solve { @@ -1440,7 +1394,24 @@ fn run_task<'a, 'b>( src, ); - Ok(()) + Ok(arena) } } } + +/// Add a task to the queue, and notify all the listeners. +fn enqueue_task<'a, 'b>( + injector: &Injector>, + listeners: &[Sender], + task: BuildTask<'a, 'b>, +) -> Result<(), LoadingProblem> { + injector.push(task); + + for listener in listeners { + listener + .send(WorkerMsg::TaskAdded) + .map_err(|_| LoadingProblem::MsgChannelDied)?; + } + + Ok(()) +} diff --git a/compiler/load/tests/test_load.rs b/compiler/load/tests/test_load.rs index 0314a7bca3..ed2b59ebe1 100644 --- a/compiler/load/tests/test_load.rs +++ b/compiler/load/tests/test_load.rs @@ -14,7 +14,6 @@ mod helpers; #[cfg(test)] mod test_load { use crate::helpers::fixtures_dir; - use bumpalo::Bump; use inlinable_string::InlinableString; use roc_can::def::Declaration::*; use roc_can::def::Def; @@ -29,17 +28,15 @@ mod test_load { // HELPERS fn load_fixture<'a>( - arena: &'a Bump, dir_name: &str, module_name: &str, subs_by_module: SubsByModule, - ) -> LoadedModule<'a> { + ) -> LoadedModule { let src_dir = fixtures_dir().join(dir_name); let filename = src_dir.join(format!("{}.roc", module_name)); let loaded = load( - &arena, &roc_builtins::std::standard_stdlib(), - src_dir, + src_dir.as_path(), filename, subs_by_module, ); @@ -131,11 +128,9 @@ mod test_load { let subs_by_module = MutMap::default(); let src_dir = fixtures_dir().join("interface_with_deps"); let filename = src_dir.join("Primary.roc"); - let arena = Bump::new(); let loaded = load( - &arena, &roc_builtins::std::standard_stdlib(), - src_dir, + src_dir.as_path(), filename, subs_by_module, ); @@ -165,9 +160,8 @@ mod test_load { #[test] fn load_unit() { - let arena = Bump::new(); let subs_by_module = MutMap::default(); - let loaded_module = load_fixture(&arena, "no_deps", "Unit", subs_by_module); + let loaded_module = load_fixture("no_deps", "Unit", subs_by_module); expect_types( loaded_module, @@ -179,10 +173,8 @@ mod test_load { #[test] fn import_alias() { - let arena = Bump::new(); let subs_by_module = MutMap::default(); - let loaded_module = - load_fixture(&arena, "interface_with_deps", "ImportAlias", subs_by_module); + let loaded_module = load_fixture("interface_with_deps", "ImportAlias", subs_by_module); expect_types( loaded_module, @@ -194,14 +186,8 @@ mod test_load { #[test] fn load_and_typecheck() { - let arena = Bump::new(); let subs_by_module = MutMap::default(); - let loaded_module = load_fixture( - &arena, - "interface_with_deps", - "WithBuiltins", - subs_by_module, - ); + let loaded_module = load_fixture("interface_with_deps", "WithBuiltins", subs_by_module); expect_types( loaded_module, @@ -220,10 +206,8 @@ mod test_load { #[test] fn iface_quicksort() { - let arena = Bump::new(); let subs_by_module = MutMap::default(); - let loaded_module = - load_fixture(&arena, "interface_with_deps", "Quicksort", subs_by_module); + let loaded_module = load_fixture("interface_with_deps", "Quicksort", subs_by_module); expect_types( loaded_module, @@ -237,9 +221,8 @@ mod test_load { #[test] fn app_quicksort() { - let arena = Bump::new(); let subs_by_module = MutMap::default(); - let loaded_module = load_fixture(&arena, "app_with_deps", "Quicksort", subs_by_module); + let loaded_module = load_fixture("app_with_deps", "Quicksort", subs_by_module); expect_types( loaded_module, @@ -253,9 +236,8 @@ mod test_load { #[test] fn load_astar() { - let arena = Bump::new(); let subs_by_module = MutMap::default(); - let loaded_module = load_fixture(&arena, "interface_with_deps", "AStar", subs_by_module); + let loaded_module = load_fixture("interface_with_deps", "AStar", subs_by_module); expect_types( loaded_module, @@ -272,9 +254,8 @@ mod test_load { #[test] fn load_principal_types() { - let arena = Bump::new(); let subs_by_module = MutMap::default(); - let loaded_module = load_fixture(&arena, "no_deps", "Principal", subs_by_module); + let loaded_module = load_fixture("no_deps", "Principal", subs_by_module); expect_types( loaded_module, @@ -287,9 +268,8 @@ mod test_load { #[test] fn iface_dep_types() { - let arena = Bump::new(); let subs_by_module = MutMap::default(); - let loaded_module = load_fixture(&arena, "interface_with_deps", "Primary", subs_by_module); + let loaded_module = load_fixture("interface_with_deps", "Primary", subs_by_module); expect_types( loaded_module, @@ -310,9 +290,8 @@ mod test_load { #[test] fn app_dep_types() { - let arena = Bump::new(); let subs_by_module = MutMap::default(); - let loaded_module = load_fixture(&arena, "app_with_deps", "Primary", subs_by_module); + let loaded_module = load_fixture("app_with_deps", "Primary", subs_by_module); expect_types( loaded_module, @@ -333,9 +312,8 @@ mod test_load { #[test] fn imported_dep_regression() { - let arena = Bump::new(); let subs_by_module = MutMap::default(); - let loaded_module = load_fixture(&arena, "interface_with_deps", "OneDep", subs_by_module); + let loaded_module = load_fixture("interface_with_deps", "OneDep", subs_by_module); expect_types( loaded_module,