Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

reentrant lock #1689

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

reentrant lock #1689

wants to merge 1 commit into from

Conversation

l3r8yJ
Copy link

@l3r8yJ l3r8yJ commented Aug 20, 2023

closes #1688

@fabriciofx take a look, please

@fabriciofx
Copy link
Contributor

@l3r8yJ

please, change the constructors to something like this:

public Synced(final Scalar<? extends T> scalar) {
   this(scalar, new ReetrantLock());
}

public Synced(final Scalar<? extends T> scalar, final ReentrantLock lock) {
   this.origin = scalar;
   this.lock = lock;
}

(and the same to Synced in Text.

Another thing: can you provide a Test to measure the performance improvement (because now is parallel)?

@victornoel
Copy link
Collaborator

victornoel commented Aug 21, 2023

Hello @l3r8yJ, what is the advantage of using this over the synchronized keyword? synchronized and reentrant locks are exactly the same semantically as far as I know.

The only difference is when using the new Java 21 virtual threads which does not support (yet) the synchronized keyword.

More details here: https://stackoverflow.com/a/11821900

@l3r8yJ
Copy link
Author

l3r8yJ commented Aug 21, 2023

@fabriciofx what's the point of these ktors? I don't see the point of this design.

can you explain, please?

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bad performance locking
3 participants