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

Cut the 0.6.0 release #86

Merged
merged 1 commit into from
Jun 6, 2018
Merged

Cut the 0.6.0 release #86

merged 1 commit into from
Jun 6, 2018

Conversation

indiv0
Copy link
Owner

@indiv0 indiv0 commented Jun 6, 2018

Update CHANGELOG.
Update README.
Cut the 0.6.0 release.

@indiv0 indiv0 self-assigned this Jun 6, 2018
Update CHANGELOG.
Update README.
Cut the 0.6.0 release.
@est31
Copy link
Contributor

est31 commented Jun 6, 2018

One question before a release: could we get a specific function? get_or_create?

@est31
Copy link
Contributor

est31 commented Jun 6, 2018

@est31
Copy link
Contributor

est31 commented Jun 6, 2018

I could whip up a PR if you want :)

@indiv0
Copy link
Owner Author

indiv0 commented Jun 6, 2018

Isn't Lazy::get_or_create equivalent to the existing LazyCell::borrow_with function?

@est31
Copy link
Contributor

est31 commented Jun 6, 2018

AtomicLazyCell doesn't seem to support that though, does it?

@indiv0
Copy link
Owner Author

indiv0 commented Jun 6, 2018

Ah yes, I'd forgotten about that. If you would be kind enough to submit a PR I'd certainly be willing to take a look :)

@est31
Copy link
Contributor

est31 commented Jun 6, 2018

Hmmm not sure how to implement borrow_with with atomics only without doing any busy waiting...

So we have the following options:

  • implement borrow_with with busy waiting
  • make addr2line single threaded for each context (would mean that every thread would require it's own context... not sure)
  • make borrow_with semi-racy so that while the function is being evaluated, the other threads compute the value themselves and return it directly (Result<&T, T> as return type for borrow_with, basically as combination of the AtomicLazyCell fill and borrow functions)

@est31
Copy link
Contributor

est31 commented Jun 6, 2018

@fitzgen @philipc @indiv0 which option should we go with?

Note that right now, std uses one global backtrace context that all threads share... Also note its totally racy way of solving the problem EDIT: redacting that, it seems to be governed by a lock in outside code.

@philipc
Copy link

philipc commented Jun 6, 2018

That lock is needed to prevent mixed output, so does that mean it won't be going away even if addr2line supports multi-threaded? If so, then I think making addr2line single threaded is ok.

For the semi-racy option, we need to make sure nothing needs to store a reference to the value. Currently FrameIter stores references to Func, but it is small enough to copy.

The busy wait and semi-racy options sound like hacks to workaround artificial issues due to libstd integration, so my preference would be to avoid them. Either the platform supports multi-threading and mutexes, and we use them, or it doesn't and we make addr2line single threaded.

Can we make the mutex support indirect? eg define a trait LazyInit { fn borrow_with(); } and change addr2line::Context to Context<R: Reader, T: LazyInit>? We can implement that trait for both LazyCell and lazy_init::Lazy, and libstd can provide its own implementation. This would also let consumers choose a LazyInit implementation that is appropriate for the Reader they are using, which may not be thread-safe. Or is that making the API too complex? Maybe go for single threaded first, and do this later if it is needed.

@est31
Copy link
Contributor

est31 commented Jun 6, 2018

If so, then I think making addr2line single threaded is ok.
[...]
Maybe go for single threaded first, and do this later if it is needed.

Sounds like a viable approach. @fitzgen what do you think? Is it OK to drop multi threading?

It seems that libstd would only need single threaded access any way due to its global lock (that lock would probably also be the place where we store the symbol context, if we can switch from sys::mutex::Mutex to sync::Mutex).

@est31
Copy link
Contributor

est31 commented Jun 6, 2018

Both the trait approach as well as the single threaded approach could use a release of lazycell in the current form. @indiv0 sorry for interrupting the release process. I think it's okay to go now :). Thanks for doing it :).

@est31 est31 mentioned this pull request Jun 6, 2018
14 tasks
@indiv0
Copy link
Owner Author

indiv0 commented Jun 6, 2018

Alrighty :)

@indiv0 indiv0 merged commit f037929 into master Jun 6, 2018
@indiv0
Copy link
Owner Author

indiv0 commented Jun 7, 2018

Version 1.0.0 has been released.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants