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

Removing cosim::utility::shared_mutex #692

Merged
merged 4 commits into from
Apr 4, 2022
Merged

Conversation

ljamt
Copy link
Member

@ljamt ljamt commented Mar 31, 2022

  • Removing the fiber friendly cosim::utility::shared_mutex
  • Also bumping version as this has not been done after 0.8.3 release

@restenb
Copy link
Member

restenb commented Mar 31, 2022

Looks good. With the removal of fibers and swap to std::mutex, are there now any changes that are necessary or beneficial to make to cosim::utility::file_lock as well?

I don't think any file operations are currently being done on the thread pool.

@@ -112,7 +49,7 @@ void file_lock::lock()
// gives us a chance to yield to the other fiber if the operation would
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment talks about fibers

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know what the correct note for lock() should be. Maybe we can use std::lock now?

There are mentions about fibers in almost every doc string in concurrenty.cpp and concurrency.hpp. @kyllingstad, shall we just remove all fiber references in the doc (remove all \pre sections) ?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In most cases, including this one, "fiber" can just be replaced by "thread". But if you want, I can take care of updating the docs in this PR.

@kyllingstad
Copy link
Member

I've pushed some doc updates now. I also took the liberty of removing some final traces of fibers in a couple of other places.

@markaren markaren self-requested a review April 4, 2022 10:11
@ljamt
Copy link
Member Author

ljamt commented Apr 4, 2022

I've pushed some doc updates now. I also took the liberty of removing some final traces of fibers in a couple of other places.

Great! Thanks a lot :)

@ljamt ljamt merged commit ecd1429 into master Apr 4, 2022
@ljamt ljamt deleted the fix/remove-cosim-sharedmutex branch April 4, 2022 10:57
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.

5 participants