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

Using ThreadLocal Digester for NameBasedGenerator to avoid synchronized #88

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

magnoyu
Copy link

@magnoyu magnoyu commented Oct 20, 2023

Hi, does it make sense to create a non synchronized version of NameBasedGenerator if the user knows the digester is thread safe? With my M1, the synchronized is about 40% more in a 10 threads scenario.

Test 'Jug, name-based, ten threads' -> 358 msecs; last UUID: 5ec5b2a6-52cc-538f-8d16-59b7565f7f78
Test 'Jug, name-based, concurrent, ten threads' -> 192 msecs; last UUID: 5ec5b2a6-52cc-538f-8d16-59b7565f7f78
Test 'Jug, name-based, ten threads' -> 359 msecs; last UUID: 5ec5b2a6-52cc-538f-8d16-59b7565f7f78
Test 'Jug, name-based, concurrent, ten threads' -> 193 msecs; last UUID: 5ec5b2a6-52cc-538f-8d16-59b7565f7f78
Test 'Jug, name-based, ten threads' -> 354 msecs; last UUID: 5ec5b2a6-52cc-538f-8d16-59b7565f7f78
Test 'Jug, name-based, concurrent, ten threads' -> 190 msecs; last UUID: 5ec5b2a6-52cc-538f-8d16-59b7565f7f78
Test 'Jug, name-based, ten threads' -> 358 msecs; last UUID: 5ec5b2a6-52cc-538f-8d16-59b7565f7f78
Test 'Jug, name-based, concurrent, ten threads' -> 197 msecs; last UUID: 5ec5b2a6-52cc-538f-8d16-59b7565f7f78
Test 'Jug, name-based, ten threads' -> 367 msecs; last UUID: 5ec5b2a6-52cc-538f-8d16-59b7565f7f78
Test 'Jug, name-based, concurrent, ten threads' -> 248 msecs; last UUID: 5ec5b2a6-52cc-538f-8d16-59b7565f7f78
Test 'Jug, name-based, ten threads' -> 383 msecs; last UUID: 5ec5b2a6-52cc-538f-8d16-59b7565f7f78
Test 'Jug, name-based, concurrent, ten threads' -> 200 msecs; last UUID: 5ec5b2a6-52cc-538f-8d16-59b7565f7f78
Test 'Jug, name-based, ten threads' -> 370 msecs; last UUID: 5ec5b2a6-52cc-538f-8d16-59b7565f7f78
Test 'Jug, name-based, concurrent, ten threads' -> 194 msecs; last UUID: 5ec5b2a6-52cc-538f-8d16-59b7565f7f78

@cowtowncoder
Copy link
Owner

General idea sounds reasonable, but I am not so sure about specific implementation. I suspect it'd be safer to use separate implementation with separate accessor for generator: keeping StringArgGenerator interface the same, but having separate implementation. And adding something like threadLocalNameBasedGenerator in Generators (or such). It might make sense to refactor common parts of 2 implementations (but not sure without looking more into this)
I think I'd like such implementation.

One future challenge is that ThreadLocal will be problematic with Project Loom where Threads will not be reused.

{
while (--rounds >= 0) {
final CyclicBarrier gate = new CyclicBarrier(11);
final MessageDigest digester = MessageDigest.getInstance("SHA-1");

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: "SHA-1" can be declared as a constant

Comment on lines +19 to +22
/**
* Method for generating name-based UUIDs using specified name (serialized to
* bytes using UTF-8 encoding). No synchronization is performed on digester.
* Digester is assumed to be created with ThreadLocal.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: It will be better if we can provide an use-case example when we should use this method.

Comment on lines +33 to +36
/**
* Method for generating name-based UUIDs using specified byte-serialization
* of name. No synchronization is performed on digester. Digester is assumed
* to be created with ThreadLocal.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same as above, can we put an example when to use this method

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.

4 participants