Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ARC API cleanup #4292

Closed
jesse99 opened this issue Dec 26, 2012 · 8 comments
Closed

ARC API cleanup #4292

jesse99 opened this issue Dec 26, 2012 · 8 comments
Labels
A-concurrency Area: Concurrency A-lifetimes Area: Lifetimes / regions C-cleanup Category: PRs that clean code up or issues documenting cleanup. E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue.

Comments

@jesse99
Copy link
Contributor

jesse99 commented Dec 26, 2012

  • ARC should have a get method to complement the get function. This would make ARCs a bit nicer to use and would be more consistent with the other arc types.
  • MutexARC should have read and write methods instead of an access method. This is more consistent with RWARC, makes the semantics of the code clearer, and is important for types like LinearMap where the API changes depending on whether the reference is immutable or not (and currently if you have a mutable reference you cannot get an immutable reference without something like the Mut type or unsafe).
@bblum
Copy link
Contributor

bblum commented Dec 26, 2012

Regarding ARC's get() function, yes, I meant to replace that with a method but never got around to it. Same for clone() and unwrap().

As for MutexARC, I'm of split minds. I called it "access" to contrast with the read/write functionality of RWARC -- no matter what you're doing with the data in a MutexARC, the lock is held in the same mode, whereas with RWARC, readers can read simultaneously. And MutexARC is unsafe, whereas RWARC is safe, so on one hand I'd say if you need different read and write functionality you should be using RWARC instead.

On the other hand, the reason MutexARC is unsafe is to allow for nesting more ARCs inside of them, which RWARCs can't do. So adding a read() function to MutexARC would allow constructing, e.g., a LinearMap in an ARC that stores more ARCs inside of it, which would be crazy, but also awesome.

Anyway, if MutexARC gains a read function, make sure the documentation stresses that it's not there for parallelism.

@jesse99
Copy link
Contributor Author

jesse99 commented Dec 27, 2012

There is probably a mismatch between what arc provides and what people want to do. For example, in my case I didn't want write access at all, but my data was not Const. So MutexARC was my only option.

@bblum
Copy link
Contributor

bblum commented Dec 30, 2012

Using MutexARC for read-only access to non-Const data strikes me as treating the symptom rather than the disease. MutexARC incurs a lot of runtime overhead (constant-time, but several pipe reads and writes) doing synchronisation, compared to ARC, which needs none at all. Instead, convert your data structure to have no mutable interior fields, and you can use ARC.

@jesse99
Copy link
Contributor Author

jesse99 commented Dec 30, 2012

Not sure it's that simple. My problem is that my struct includes closures and closures are not Const (see #3569).

@bblum
Copy link
Contributor

bblum commented Dec 30, 2012

Then that is that bug's problem, not this one. ;)

@emberian
Copy link
Member

emberian commented Jul 7, 2013

@bblum still relevant? It seems the API has already been changed to use methods

@bblum
Copy link
Contributor

bblum commented Jul 7, 2013

A little bit. The constructors still need to be changed from e.g. RWARC() to RWARC::new().

@catamorphism
Copy link
Contributor

The constructors have now been fixed. I think this bug can be closed; if you know of anything else that needs to be done, please open up a more specific bug.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-concurrency Area: Concurrency A-lifetimes Area: Lifetimes / regions C-cleanup Category: PRs that clean code up or issues documenting cleanup. E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue.
Projects
None yet
Development

No branches or pull requests

4 participants