You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@pekko.apache.org by "pjfanning (via GitHub)" <gi...@apache.org> on 2024/02/01 12:04:11 UTC
[PR] use ReentrantLock in LockUtil (Switch) [incubator-pekko]
pjfanning opened a new pull request, #1092:
URL: https://github.com/apache/incubator-pekko/pull/1092
an example change for #1089
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
Re: [PR] use ReentrantLock in LockUtil (Switch) [incubator-pekko]
Posted by "mdedetrich (via GitHub)" <gi...@apache.org>.
mdedetrich commented on PR #1092:
URL: https://github.com/apache/incubator-pekko/pull/1092#issuecomment-1924928551
> I'm open to abandoning this. But if we abandon this, I think that #1089 should be changed to not be a 'good first issue'. It isn't a good first issue if it needs serious expertise to decide on which 'synchronized' blocks need to be changed.
>
> To flip this on it's head, maybe we need to get people to use Pekko with virtual threads and to report to us the pinned thread issues that they hit.
Completely agree with this, `VirtualThread` support is not a good first issue not only due to it deceptively easy but also because since its a JDK 21 feature as is well known we are having issues on whats the best way to integrate this into Pekko.
I am still heavily in the "lets just document how JDK 21 users can create their own VirtualThreadPool" camp rather than pushing these kind of changes in.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
Re: [PR] use ReentrantLock in LockUtil (Switch) [incubator-pekko]
Posted by "pjfanning (via GitHub)" <gi...@apache.org>.
pjfanning commented on PR #1092:
URL: https://github.com/apache/incubator-pekko/pull/1092#issuecomment-1921313043
I open to abandoning this. But if we abandon this, I think that #1089 should be be changed to not be a 'good first issue'. It isn't a good first issue if it needs serious expertise to decide on which 'synchronized' blocks need to be changed.
To flip this on it's head, maybe we need to get people to use Pekko with virtual threads and to report to us the pinned thread issues that they hit.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
Re: [PR] use ReentrantLock in LockUtil (Switch) [incubator-pekko]
Posted by "laglangyue (via GitHub)" <gi...@apache.org>.
laglangyue commented on PR #1092:
URL: https://github.com/apache/incubator-pekko/pull/1092#issuecomment-1921196712
lgtm
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
Re: [PR] use ReentrantLock in LockUtil (Switch) [incubator-pekko]
Posted by "He-Pin (via GitHub)" <gi...@apache.org>.
He-Pin commented on PR #1092:
URL: https://github.com/apache/incubator-pekko/pull/1092#issuecomment-1921290345
I think we should only do this where io is involved.
The only issue of the current implementation is it introduce some allocation with the by name parameter.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
Re: [PR] use ReentrantLock in LockUtil (Switch) [incubator-pekko]
Posted by "He-Pin (via GitHub)" <gi...@apache.org>.
He-Pin commented on PR #1092:
URL: https://github.com/apache/incubator-pekko/pull/1092#issuecomment-1921340927
I will back my hometown tomorrow, I will try to work on the virtual thread support.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
Re: [PR] use ReentrantLock in LockUtil (Switch) [incubator-pekko]
Posted by "pjfanning (via GitHub)" <gi...@apache.org>.
pjfanning commented on PR #1092:
URL: https://github.com/apache/incubator-pekko/pull/1092#issuecomment-1925250689
@mdedetrich this change does not use any new classes - it uses existing classes. `synchronized` has not been improved in Java 21 but locks like ReentrantLock have been. ReentrantLock does not block the carrier thread and other virtual threads can use the carrier thread while the lock is waiting. `synchronized` does block the carrier thread and other virtual threads will not get a chance to use the carrier thread during the synchronized blocking is waiting.
See https://softwaremill.com/what-is-blocking-in-loom/#retrofitting-blocking
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
Re: [PR] use ReentrantLock in LockUtil (Switch) [incubator-pekko]
Posted by "pjfanning (via GitHub)" <gi...@apache.org>.
pjfanning commented on PR #1092:
URL: https://github.com/apache/incubator-pekko/pull/1092#issuecomment-1921284709
I won't merge this yet but I'd appreciate some feedback. With Loom in mind, there doesn't appear to be a big reason not to start changing synchronized blocks to ReentrantLocks. There is a fair amount of documentation online encouraging this change.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
Re: [PR] use ReentrantLock in LockUtil (Switch) [incubator-pekko]
Posted by "mdedetrich (via GitHub)" <gi...@apache.org>.
mdedetrich commented on PR #1092:
URL: https://github.com/apache/incubator-pekko/pull/1092#issuecomment-1925285219
> @mdedetrich this change does not use any new classes - it uses existing classes. `synchronized` has not been improved in Java 21 but locks like ReentrantLock have been. ReentrantLock does not block the carrier thread and other virtual threads can use the carrier thread while the lock is waiting. `synchronized` does block the carrier thread and other virtual threads will not get a chance to use the carrier thread during the synchronized blocking is waiting (known as pinning).
>
> See https://softwaremill.com/what-is-blocking-in-loom/#retrofitting-blocking
I wasn't talking about this PR but the other one with VirtualThreadPool.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org