Remove outdated doccomment on EngineState (#12158)

Part of the doccomment was an implementation note on the `im` crate that
hasn't been used for ages.
(If I recall we maybe even received a comment on discord on this)
This commit is contained in:
Stefan Holderbach 2024-03-11 15:57:28 +01:00 committed by GitHub
parent f6853fd636
commit 77379d7b3d
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 1 additions and 33 deletions

View file

@ -67,37 +67,6 @@ impl Clone for IsDebugging {
/// ///
/// Note that the runtime stack is not part of this global state. Runtime stacks are handled differently, /// Note that the runtime stack is not part of this global state. Runtime stacks are handled differently,
/// but they also rely on using IDs rather than full definitions. /// but they also rely on using IDs rather than full definitions.
///
/// A note on implementation:
///
/// Much of the global definitions are built on the Bodil's 'im' crate. This gives us a way of working with
/// lists of definitions in a way that is very cheap to access, while also allowing us to update them at
/// key points in time (often, the transition between parsing and evaluation).
///
/// Over the last two years we tried a few different approaches to global state like this. I'll list them
/// here for posterity, so we can more easily know how we got here:
///
/// * `Rc` - Rc is cheap, but not thread-safe. The moment we wanted to work with external processes, we
/// needed a way send to stdin/stdout. In Rust, the current practice is to spawn a thread to handle both.
/// These threads would need access to the global state, as they'll need to process data as it streams out
/// of the data pipeline. Because Rc isn't thread-safe, this breaks.
///
/// * `Arc` - Arc is the thread-safe version of the above. Often Arc is used in combination with a Mutex or
/// RwLock, but you can use Arc by itself. We did this a few places in the original Nushell. This *can* work
/// but because of Arc's nature of not allowing mutation if there's a second copy of the Arc around, this
/// ultimately becomes limiting.
///
/// * `Arc` + `Mutex/RwLock` - the standard practice for thread-safe containers. Unfortunately, this would
/// have meant we would incur a lock penalty every time we needed to access any declaration or block. As we
/// would be reading far more often than writing, it made sense to explore solutions that favor large amounts
/// of reads.
///
/// * `im` - the `im` crate was ultimately chosen because it has some very nice properties: it gives the
/// ability to cheaply clone these structures, which is nice as EngineState may need to be cloned a fair bit
/// to follow ownership rules for closures and iterators. It also is cheap to access. Favoring reads here fits
/// more closely to what we need with Nushell. And, of course, it's still thread-safe, so we get the same
/// benefits as above.
///
#[derive(Clone)] #[derive(Clone)]
pub struct EngineState { pub struct EngineState {
files: Vec<(String, usize, usize)>, files: Vec<(String, usize, usize)>,

View file

@ -49,8 +49,7 @@ impl std::fmt::Debug for CliError<'_> {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
let config = self.1.get_config(); let config = self.1.get_config();
let ansi_support = &config.use_ansi_coloring; let ansi_support = config.use_ansi_coloring;
let ansi_support = *ansi_support;
let error_style = &config.error_style; let error_style = &config.error_style;