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

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?

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.

Bad performance locking
3 participants