Cargo::util::config::* - why all the Cells?


#1

In cargo::util::config::Config, lots of fields are cells. I was wondering what the reason is for this - why can’t methods that update fields just take &mut self?


#2

I don’t think there’s any real reason behind this except “it used to be convenient”. I think Config::configure is the only method that needs to mutate most of this fields, and it is only called inside execute at src/bin/cargo.rs:149.

It might be interesting to try to get rid of those cells and see if it works!


#3

I’ve had more of a look, and it seems that having interior mutability here means you can pass Config around all over the place, without worrying about having more than 1 mutable reference. I think it should still not have interior mutability, as it obscures what is happening, so I’ll see if it’s fesible to change.


#4

Yeah, I think some of thous cells are mutated down the line somhere, but there’s a high change that &mut will suffice for most of them!


#5

I’ve successfully removed a few of the Cells and all tests pass, but now I need to decide what to do with the remaining ones, especially LazyCells. Currently they are lazily initialized, which improves performance if they are never used (to initialize rustc, values, cargo_exe, rustdoc requires a filesystem call, to initialize easy requires loading a shared object file).

I could just load these eagerly, or load them in a worker thread, or leave them unchanged. What do you think is best?


#6

I think LazyCell is a great abstraction, actually, because it allows to defer some work without introducing mutabilty and reference wrappers, like Ref of RefCell, so I think it’s best to leave it as is.

It would be great to switch to https://github.com/indiv0/lazycell, but we’ll need a couple of additional APIs for that: https://github.com/indiv0/lazycell/pulls.


#7

I’ve submitted a PR just for the Cells