comparison rust/hg-core/src/repo.rs @ 49177:90a15199cbc6

rust-repo: make `Send` by not storing functions in `LazyCell` We (Google) want to use `Repo` in a context where we can store it in `Mutex<Repo>`. However, that currently doesn't work because it's not `Send` because the `LazyCell` initialization functions are not `Send`. It's easy to fix that by passing them to the `get_or_init()` and `get_mut_or_init()` functions. We'll probably also want `Repo` to be `Send` (and even `Sync`) in core later, so this seems like a step in the right direction. Differential Revision: https://phab.mercurial-scm.org/D12582
author Martin von Zweigbergk <martinvonz@google.com>
date Thu, 21 Apr 2022 10:39:52 -0700
parents a932cad26d37
children 13dfad0f9f7a
comparison
equal deleted inserted replaced
49176:3cd3aaba5b03 49177:90a15199cbc6
27 working_directory: PathBuf, 27 working_directory: PathBuf,
28 dot_hg: PathBuf, 28 dot_hg: PathBuf,
29 store: PathBuf, 29 store: PathBuf,
30 requirements: HashSet<String>, 30 requirements: HashSet<String>,
31 config: Config, 31 config: Config,
32 dirstate_parents: LazyCell<DirstateParents, HgError>, 32 dirstate_parents: LazyCell<DirstateParents>,
33 dirstate_data_file_uuid: LazyCell<Option<Vec<u8>>, HgError>, 33 dirstate_data_file_uuid: LazyCell<Option<Vec<u8>>>,
34 dirstate_map: LazyCell<OwningDirstateMap, DirstateError>, 34 dirstate_map: LazyCell<OwningDirstateMap>,
35 changelog: LazyCell<Changelog, HgError>, 35 changelog: LazyCell<Changelog>,
36 manifestlog: LazyCell<Manifestlog, HgError>, 36 manifestlog: LazyCell<Manifestlog>,
37 } 37 }
38 38
39 #[derive(Debug, derive_more::From)] 39 #[derive(Debug, derive_more::From)]
40 pub enum RepoError { 40 pub enum RepoError {
41 NotFound { 41 NotFound {
180 requirements: reqs, 180 requirements: reqs,
181 working_directory, 181 working_directory,
182 store: store_path, 182 store: store_path,
183 dot_hg, 183 dot_hg,
184 config: repo_config, 184 config: repo_config,
185 dirstate_parents: LazyCell::new(Self::read_dirstate_parents), 185 dirstate_parents: LazyCell::new(),
186 dirstate_data_file_uuid: LazyCell::new( 186 dirstate_data_file_uuid: LazyCell::new(),
187 Self::read_dirstate_data_file_uuid, 187 dirstate_map: LazyCell::new(),
188 ), 188 changelog: LazyCell::new(),
189 dirstate_map: LazyCell::new(Self::new_dirstate_map), 189 manifestlog: LazyCell::new(),
190 changelog: LazyCell::new(Self::new_changelog),
191 manifestlog: LazyCell::new(Self::new_manifestlog),
192 }; 190 };
193 191
194 requirements::check(&repo)?; 192 requirements::check(&repo)?;
195 193
196 Ok(repo) 194 Ok(repo)
258 .io_not_found_as_none()? 256 .io_not_found_as_none()?
259 .unwrap_or(Vec::new())) 257 .unwrap_or(Vec::new()))
260 } 258 }
261 259
262 pub fn dirstate_parents(&self) -> Result<DirstateParents, HgError> { 260 pub fn dirstate_parents(&self) -> Result<DirstateParents, HgError> {
263 Ok(*self.dirstate_parents.get_or_init(self)?) 261 Ok(*self
262 .dirstate_parents
263 .get_or_init(|| self.read_dirstate_parents())?)
264 } 264 }
265 265
266 fn read_dirstate_parents(&self) -> Result<DirstateParents, HgError> { 266 fn read_dirstate_parents(&self) -> Result<DirstateParents, HgError> {
267 let dirstate = self.dirstate_file_contents()?; 267 let dirstate = self.dirstate_file_contents()?;
268 let parents = if dirstate.is_empty() { 268 let parents = if dirstate.is_empty() {
338 } 338 }
339 339
340 pub fn dirstate_map( 340 pub fn dirstate_map(
341 &self, 341 &self,
342 ) -> Result<Ref<OwningDirstateMap>, DirstateError> { 342 ) -> Result<Ref<OwningDirstateMap>, DirstateError> {
343 self.dirstate_map.get_or_init(self) 343 self.dirstate_map.get_or_init(|| self.new_dirstate_map())
344 } 344 }
345 345
346 pub fn dirstate_map_mut( 346 pub fn dirstate_map_mut(
347 &self, 347 &self,
348 ) -> Result<RefMut<OwningDirstateMap>, DirstateError> { 348 ) -> Result<RefMut<OwningDirstateMap>, DirstateError> {
349 self.dirstate_map.get_mut_or_init(self) 349 self.dirstate_map
350 .get_mut_or_init(|| self.new_dirstate_map())
350 } 351 }
351 352
352 fn new_changelog(&self) -> Result<Changelog, HgError> { 353 fn new_changelog(&self) -> Result<Changelog, HgError> {
353 Changelog::open(&self.store_vfs(), self.has_nodemap()) 354 Changelog::open(&self.store_vfs(), self.has_nodemap())
354 } 355 }
355 356
356 pub fn changelog(&self) -> Result<Ref<Changelog>, HgError> { 357 pub fn changelog(&self) -> Result<Ref<Changelog>, HgError> {
357 self.changelog.get_or_init(self) 358 self.changelog.get_or_init(|| self.new_changelog())
358 } 359 }
359 360
360 pub fn changelog_mut(&self) -> Result<RefMut<Changelog>, HgError> { 361 pub fn changelog_mut(&self) -> Result<RefMut<Changelog>, HgError> {
361 self.changelog.get_mut_or_init(self) 362 self.changelog.get_mut_or_init(|| self.new_changelog())
362 } 363 }
363 364
364 fn new_manifestlog(&self) -> Result<Manifestlog, HgError> { 365 fn new_manifestlog(&self) -> Result<Manifestlog, HgError> {
365 Manifestlog::open(&self.store_vfs(), self.has_nodemap()) 366 Manifestlog::open(&self.store_vfs(), self.has_nodemap())
366 } 367 }
367 368
368 pub fn manifestlog(&self) -> Result<Ref<Manifestlog>, HgError> { 369 pub fn manifestlog(&self) -> Result<Ref<Manifestlog>, HgError> {
369 self.manifestlog.get_or_init(self) 370 self.manifestlog.get_or_init(|| self.new_manifestlog())
370 } 371 }
371 372
372 pub fn manifestlog_mut(&self) -> Result<RefMut<Manifestlog>, HgError> { 373 pub fn manifestlog_mut(&self) -> Result<RefMut<Manifestlog>, HgError> {
373 self.manifestlog.get_mut_or_init(self) 374 self.manifestlog.get_mut_or_init(|| self.new_manifestlog())
374 } 375 }
375 376
376 /// Returns the manifest of the *changeset* with the given node ID 377 /// Returns the manifest of the *changeset* with the given node ID
377 pub fn manifest_for_node( 378 pub fn manifest_for_node(
378 &self, 379 &self,
422 let map = self.dirstate_map()?; 423 let map = self.dirstate_map()?;
423 // TODO: Maintain a `DirstateMap::dirty` flag, and return early here if 424 // TODO: Maintain a `DirstateMap::dirty` flag, and return early here if
424 // it’s unset 425 // it’s unset
425 let parents = self.dirstate_parents()?; 426 let parents = self.dirstate_parents()?;
426 let (packed_dirstate, old_uuid_to_remove) = if self.has_dirstate_v2() { 427 let (packed_dirstate, old_uuid_to_remove) = if self.has_dirstate_v2() {
427 let uuid_opt = self.dirstate_data_file_uuid.get_or_init(self)?; 428 let uuid_opt = self
429 .dirstate_data_file_uuid
430 .get_or_init(|| self.read_dirstate_data_file_uuid())?;
428 let uuid_opt = uuid_opt.as_ref(); 431 let uuid_opt = uuid_opt.as_ref();
429 let can_append = uuid_opt.is_some(); 432 let can_append = uuid_opt.is_some();
430 let (data, tree_metadata, append, old_data_size) = 433 let (data, tree_metadata, append, old_data_size) =
431 map.pack_v2(can_append)?; 434 map.pack_v2(can_append)?;
432 435
506 } 509 }
507 510
508 /// Lazily-initialized component of `Repo` with interior mutability 511 /// Lazily-initialized component of `Repo` with interior mutability
509 /// 512 ///
510 /// This differs from `OnceCell` in that the value can still be "deinitialized" 513 /// This differs from `OnceCell` in that the value can still be "deinitialized"
511 /// later by setting its inner `Option` to `None`. 514 /// later by setting its inner `Option` to `None`. It also takes the
512 struct LazyCell<T, E> { 515 /// initialization function as an argument when the value is requested, not
516 /// when the instance is created.
517 struct LazyCell<T> {
513 value: RefCell<Option<T>>, 518 value: RefCell<Option<T>>,
514 // `Fn`s that don’t capture environment are zero-size, so this box does
515 // not allocate:
516 init: Box<dyn Fn(&Repo) -> Result<T, E>>,
517 } 519 }
518 520
519 impl<T, E> LazyCell<T, E> { 521 impl<T> LazyCell<T> {
520 fn new(init: impl Fn(&Repo) -> Result<T, E> + 'static) -> Self { 522 fn new() -> Self {
521 Self { 523 Self {
522 value: RefCell::new(None), 524 value: RefCell::new(None),
523 init: Box::new(init),
524 } 525 }
525 } 526 }
526 527
527 fn set(&self, value: T) { 528 fn set(&self, value: T) {
528 *self.value.borrow_mut() = Some(value) 529 *self.value.borrow_mut() = Some(value)
529 } 530 }
530 531
531 fn get_or_init(&self, repo: &Repo) -> Result<Ref<T>, E> { 532 fn get_or_init<E>(
533 &self,
534 init: impl Fn() -> Result<T, E>,
535 ) -> Result<Ref<T>, E> {
532 let mut borrowed = self.value.borrow(); 536 let mut borrowed = self.value.borrow();
533 if borrowed.is_none() { 537 if borrowed.is_none() {
534 drop(borrowed); 538 drop(borrowed);
535 // Only use `borrow_mut` if it is really needed to avoid panic in 539 // Only use `borrow_mut` if it is really needed to avoid panic in
536 // case there is another outstanding borrow but mutation is not 540 // case there is another outstanding borrow but mutation is not
537 // needed. 541 // needed.
538 *self.value.borrow_mut() = Some((self.init)(repo)?); 542 *self.value.borrow_mut() = Some(init()?);
539 borrowed = self.value.borrow() 543 borrowed = self.value.borrow()
540 } 544 }
541 Ok(Ref::map(borrowed, |option| option.as_ref().unwrap())) 545 Ok(Ref::map(borrowed, |option| option.as_ref().unwrap()))
542 } 546 }
543 547
544 fn get_mut_or_init(&self, repo: &Repo) -> Result<RefMut<T>, E> { 548 fn get_mut_or_init<E>(
549 &self,
550 init: impl Fn() -> Result<T, E>,
551 ) -> Result<RefMut<T>, E> {
545 let mut borrowed = self.value.borrow_mut(); 552 let mut borrowed = self.value.borrow_mut();
546 if borrowed.is_none() { 553 if borrowed.is_none() {
547 *borrowed = Some((self.init)(repo)?); 554 *borrowed = Some(init()?);
548 } 555 }
549 Ok(RefMut::map(borrowed, |option| option.as_mut().unwrap())) 556 Ok(RefMut::map(borrowed, |option| option.as_mut().unwrap()))
550 } 557 }
551 } 558 }