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

Add support for virtual threads: replace synchronized with ReentrantLock #860

Closed
wants to merge 1 commit into from

Conversation

bdeneuter
Copy link

Add support for virtual threads which was added to JDK 19: replace synchronized with ReentrantLock.

The use of ReentrantLock avoids that the carrier thread gets pinned.

See https://openjdk.org/jeps/425:
The scheduler does not compensate for pinning by expanding its parallelism. Instead, avoid frequent and long-lived pinning by revising synchronized blocks or methods that run frequently and guard potentially long I/O operations to use java.util.concurrent.locks.ReentrantLock instead

…ock.

This avoids that the carrier thread gets pinned.
See https://openjdk.org/jeps/425:
`The scheduler does not compensate for pinning by expanding its parallelism. Instead, avoid frequent and long-lived pinning by revising synchronized blocks or methods that run frequently and guard potentially long I/O operations to use java.util.concurrent.locks.ReentrantLock instead`
@CLAassistant
Copy link

CLAassistant commented Jan 14, 2023

CLA assistant check
All committers have signed the CLA.

@ben-manes
Copy link
Owner

Thanks for giving it a try, but this won't work as you are expecting. I'm going to close for now.

For the immediate term, AsyncCache is virtual threads friendly because the computation occurs within a CompletableFuture. Then the synchronizations do not perform any I/O, which is acceptable usages for the current implementation.

The underlying ConcurrentHashMap compute methods use synchronization for locking, so a synchronous cache is incompatible because of that locking behavior regardless of our own. The JEP authors have not rewritten the hash table and last time that I discussed this with Doug Lea (July 2022) was only in an idea state. Similarly, for our own usages we do not want to pay the memory cost of a lock per entry.

"One of these days I plan to give another shot at avoiding builtin monitor locks... keeping a separate bitwise-trie-like structure of AQS objects that serve as the inflated forms. One TBD is how/when to kill them off."

The JEP authors have stated that fixing synchronization is planned before the feature is promoted out of its preview status. At the time it was estimated to occur with the 3rd preview, which would mean jdk 21. This limitation was therefore communicated for experimenting, providing feedback, and adapting if trivial to do so. It is not meant to be a long-term restriction.

For now it appears that the best approach is to simply wait. If virtual threads keeps this limitation long-term and the JDK revamps its usages then we'll learn from their tricks and try to do likewise.

@ben-manes ben-manes closed this Jan 14, 2023
@bdeneuter
Copy link
Author

Thx @ben-manes for the valuable feedback. I was trying to find out if/when they were planning to fix the synchronization but didn't find any public statement about it.

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.

None yet

4 participants