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

Striped fiber callback hashtable #1913

Merged
merged 13 commits into from
Apr 24, 2021
Merged

Striped fiber callback hashtable #1913

merged 13 commits into from
Apr 24, 2021

Conversation

vasilmkd
Copy link
Member

@vasilmkd vasilmkd commented Apr 20, 2021

Cats Effect 3 IO has a mechanism for registering the error callback (Throwable => Unit) of each Fiber which is unsafeRun*. These callbacks are important for the runtime to have control over and this feature enables proper error reporting during an abrupt shutdown of the IORuntime (usually due to unrecoverable VM errors).

Up to now, we had a single thread safe hashtable which was locked exclusively by each unsafeRun* operation, both when starting and when ending a fiber (inserting the callback and removing it). While these locks were only meant for achieving mutual exclusion in a very tiny critical section (there is no possibility of deadlocks or anything), this is still a bottleneck in a system where a lot of IOs are directly unsafeRun*, possibly concurrently.

This PR is very similar in spirit to the ScalQueue PR and how java.util.concurrent.ConcurrentHashMap works. It introduces a new internal type which conceptually represents a thread safe hashtable, but is in fact composed of many of the previously developed thread safe hashtables, and load balances between them. This way, while each subsection of the hashtable is still locked exclusively during insertion and removal, it reduces the contention on each lock because there are many more of them, instead of 1 global lock, and presumably the hashes of the callbacks are regularly distributed and map to different subsections of the whole hashtable.

The code only touches internal classes and should be completely binary compatible (with added filters), but please review this carefully, I'm still new to mima.

@vasilmkd vasilmkd marked this pull request as ready for review April 20, 2021 07:52
@djspiewak
Copy link
Member

Silly question: why not just use ConcurrentHashMap?

@vasilmkd
Copy link
Member Author

vasilmkd commented Apr 20, 2021

It's not a silly question. We developed this custom hashtable to be able to iterate over it without allocation of any objects, which is important when a VM error occurs, we catch it in order to notify every fiber, but during this time, the VM is in an undefined state. Is this still desirable?

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.

2 participants