Skip to content

Commit

Permalink
fix: add Session::enter_parallel
Browse files Browse the repository at this point in the history
Keep previous behavior for `enter`, add a separate function to
install thread pool.
  • Loading branch information
DaniPopes committed Jan 5, 2025
1 parent 84f374e commit 03316a3
Show file tree
Hide file tree
Showing 5 changed files with 58 additions and 11 deletions.
2 changes: 1 addition & 1 deletion crates/cli/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ fn run_compiler_with(opts: Opts, f: impl FnOnce(&Compiler) -> Result + Send) ->
sess.validate()?;

let compiler = Compiler { sess };
compiler.sess.enter(|| {
compiler.sess.enter_parallel(|| {
let mut r = f(&compiler);
r = compiler.finish_diagnostics().and(r);
r
Expand Down
12 changes: 12 additions & 0 deletions crates/interface/src/globals.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,18 @@ impl SessionGlobals {
/// Panics if `set` has not previously been called.
#[inline]
pub fn with<R>(f: impl FnOnce(&Self) -> R) -> R {
#[cfg(debug_assertions)]
if !SESSION_GLOBALS.is_set() {
let msg = if rayon::current_thread_index().is_some() {
"cannot access a scoped thread local variable without calling `set` first;\n\
did you forget to call `Session::enter_parallel`?"
} else {
"cannot access a scoped thread local variable without calling `set` first;\n\
did you forget to call `Session::enter`, or `Session::enter_parallel` \
if using Rayon?"
};
panic!("{msg}");
}
SESSION_GLOBALS.with(f)
}

Expand Down
38 changes: 29 additions & 9 deletions crates/interface/src/session.rs
Original file line number Diff line number Diff line change
Expand Up @@ -265,12 +265,26 @@ impl Session {
solar_data_structures::sync::scope(self.is_parallel(), op)
}

/// Sets up the thread pool and session globals if they doesn't exist already and then
/// executes the given closure.
/// Sets up the session globals if they doesn't exist already and then executes the given
/// closure.
///
/// Note that this does not set up the rayon thread pool. This is only useful when parsing
/// sequentially, like manually using `Parser`.
///
/// This also calls [`SessionGlobals::with_source_map`].
#[inline]
pub fn enter<R: Send>(&self, f: impl FnOnce() -> R + Send) -> R {
SessionGlobals::with_or_default(|_| {
SessionGlobals::with_source_map(self.clone_source_map(), f)
})
}

/// Sets up the thread pool and session globals if they doesn't exist already and then executes
/// the given closure.
///
/// This also calls [`SessionGlobals::with_source_map`].
#[inline]
pub fn enter_parallel<R: Send>(&self, f: impl FnOnce() -> R + Send) -> R {
SessionGlobals::with_or_default(|session_globals| {
SessionGlobals::with_source_map(self.clone_source_map(), || {
run_in_thread_pool_with_globals(self.threads(), session_globals, f)
Expand All @@ -287,6 +301,10 @@ fn run_in_thread_pool_with_globals<R: Send>(
) -> R {
// Avoid panicking below if this is a recursive call.
if rayon::current_thread_index().is_some() {
debug!(
"running in the current thread's rayon thread pool; \
this could cause panics later on if it was created without setting the session globals!"
);
return f();
}

Expand Down Expand Up @@ -380,25 +398,27 @@ mod tests {
assert!(!s.contains("Span("), "{s}");
let s = format!("{span:#?}");
assert!(!s.contains("Span("), "{s}");

assert!(rayon::current_thread_index().is_some());
}

let sess = Session::builder().with_buffer_emitter(ColorChoice::Never).build();
sess.enter(use_globals);
sess.enter_parallel(use_globals);
assert!(sess.dcx.emitted_diagnostics().unwrap().is_empty());
assert!(sess.dcx.emitted_errors().unwrap().is_ok());
sess.enter(|| {
sess.enter_parallel(|| {
use_globals();
sess.enter(use_globals);
sess.enter_parallel(use_globals);
use_globals();
});
assert!(sess.dcx.emitted_diagnostics().unwrap().is_empty());
assert!(sess.dcx.emitted_errors().unwrap().is_ok());

SessionGlobals::new().set(|| {
use_globals_no_sm();
sess.enter(|| {
sess.enter_parallel(|| {
use_globals();
sess.enter(use_globals);
sess.enter_parallel(use_globals);
use_globals();
});
use_globals_no_sm();
Expand All @@ -411,12 +431,12 @@ mod tests {
fn enter_diags() {
let sess = Session::builder().with_buffer_emitter(ColorChoice::Never).build();
assert!(sess.dcx.emitted_errors().unwrap().is_ok());
sess.enter(|| {
sess.enter_parallel(|| {
sess.dcx.err("test1").emit();
assert!(sess.dcx.emitted_errors().unwrap().is_err());
});
assert!(sess.dcx.emitted_errors().unwrap().unwrap_err().to_string().contains("test1"));
sess.enter(|| {
sess.enter_parallel(|| {
sess.dcx.err("test2").emit();
assert!(sess.dcx.emitted_errors().unwrap().is_err());
});
Expand Down
2 changes: 2 additions & 0 deletions crates/parse/src/lexer/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,8 @@ impl<'sess, 'src> Lexer<'sess, 'src> {
}

/// Creates a new `Lexer` for the given source file.
///
/// Note that the source file must be added to the source map before calling this function.
pub fn from_source_file(sess: &'sess Session, file: &'src SourceFile) -> Self {
Self::with_start_pos(sess, &file.src, file.start_pos)
}
Expand Down
15 changes: 14 additions & 1 deletion crates/parse/src/parser/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -147,13 +147,22 @@ impl<'sess, 'ast> Parser<'sess, 'ast> {
}

/// Creates a new parser from a file.
///
/// The file will not be read if it has already been added into the source map.
pub fn from_file(sess: &'sess Session, arena: &'ast ast::Arena, path: &Path) -> Result<Self> {
Self::from_lazy_source_code(sess, arena, FileName::Real(path.to_path_buf()), || {
std::fs::read_to_string(path)
std::fs::read_to_string(path).map_err(|e| {
std::io::Error::new(
e.kind(),
solar_interface::source_map::ResolveError::ReadFile(path.to_path_buf(), e),
)
})
})
}

/// Creates a new parser from a source code closure.
///
/// The closure will not be called if the file name has already been added into the source map.
pub fn from_lazy_source_code(
sess: &'sess Session,
arena: &'ast ast::Arena,
Expand All @@ -168,6 +177,10 @@ impl<'sess, 'ast> Parser<'sess, 'ast> {
}

/// Creates a new parser from a source file.
///
/// Note that the source file must be added to the source map before calling this function.
/// Prefer using [`from_source_code`](Self::from_source_code) or [`from_file`](Self::from_file)
/// instead.
pub fn from_source_file(
sess: &'sess Session,
arena: &'ast ast::Arena,
Expand Down

0 comments on commit 03316a3

Please sign in to comment.