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

derekdreery · 2017-10-01T13:42:16.257Z

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?

matklad · 2017-10-01T13:46:12.074Z

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!

derekdreery · 2017-10-03T11:18:49.538Z

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.

matklad · 2017-10-03T11:20:23.906Z

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!

derekdreery · 2017-10-04T17:25:19.762Z

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?

matklad · 2017-10-04T18:07:32.682Z

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.

derekdreery · 2017-10-04T18:18:40.859Z

I’ve submitted a PR just for the Cells

system · 2019-03-25T08:29:04.161Z

This topic was automatically closed 90 days after the last reply. New replies are no longer allowed.