Hi Murray,
broadly speaking, synchronized blocks aren't go to play so very nice
with the new virtual threads functionality, so in order to take
advantage of them, the suggeston is to switch to something else,
namely locking with a ReentrantLock
Going that way, the PR ends up having a lot of blocks like
> lock.lock();
> try {
> doSomething();
> } finally {
> lock.unlock();
> }
instead of having that everytime, I suggested encapsulating that block
on an utility method, so instead of that you simply end up writting
something like
> Synchronizer.synchronize( lock, ()-> doSomething() );
The utility class is just to capture the idiom that is getting
repeated throughout the code, not managing or trying to enhance the
lock in any way. And of course, if later on more advanced cases of
locking/syncing appear, we can move away from the utility method, and
we also don't have to tie to it later on if it's not needed.
cheers,
juan pablo
On Mon, Oct 9, 2023 at 12:22 PM Murray Altheim <murra...@altheim.com> wrote:
>
> Hi,
>
> My apologies for arriving a bit out of context, so please excuse me if this
> has already been asked or addressed, but can I assume that we would be using
> the existing java.util.concurrent.locks.ReentrantLock class for this? I'm
> trying to understand the question better. I don't see the need for a utility
> class if ReentrantLock is used, and would advise against a bespoke solution
> considering the JVM is likely optimised to handle the existing class.
>
> I'm not yet familiar with JDK 21's Virtual Threads and am just reading up
> on them now.
>
> Cheers,
>
> Murray
>
> On 9/10/23 22:05, juanpablo-santos (via GitHub) wrote:
> >
> > juanpablo-santos commented on PR #307:
> > URL: https://github.com/apache/jspwiki/pull/307#issuecomment-1752606848
> >
> > Hi @arturobernalg !
> >
> > > However, this approach has limitations when it comes to working with
> > condition variables and allowing for custom scenarios. Specifically, using
> > a utility class for locking would make it challenging to implement more
> > complex control flows that involve waiting for certain conditions to be met
> > or signaling other threads to proceed.
> > >
> > > In essence, while the utility class would make the code cleaner for
> > basic locking and unlocking, it might not be flexible enough to handle
> > advanced locking scenarios that require the use of conditions.
> >
> > I agree that more complex scenarios would fit inside this utility
> > class. However, the scope of the PR is to switch away from `synchronized`
> > blocks, and for all them we have the same idiom all over the place:
> >
> > ```
> > lock.lock();
> > try {
> > doSomething();
> > } finally {
> > lock.unlock();
> > }
> > ```
> >
> > So abstracting it into a common method makes sense to me. If later on a
> > we want to refactor or we want to capture more complex scenarios, we can
> > move away from the utility `synchronize` method. The utility/class method
> > focus should be more about synchronizing than locking (hence the names).
> >
> > WDYT?
> >
> >
>
> --
>
> ...........................................................................
> Murray Altheim <murray18 at altheim dot com> = = ===
> http://www.altheim.com/murray/ === ===
> = = ===
> In the evening
> The rice leaves in the garden
> Rustle in the autumn wind
> That blows through my reed hut.
> -- Minamoto no Tsunenobu
>
