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

Try to fix the synchronisation of the language detection singleton in… #585

Merged
merged 1 commit into from May 30, 2020

Conversation

lfoppiano
Copy link
Collaborator

…itialisation in case of multithread environment.

There are many different theories around this, but basically what I understood is that

  • double null check + synchronize + volatile correctly works, so thread are locked after null checking the instance... but a secondary check avoid threads that have passed the null check to repeat the initialisation.
  • not sure it's faster than just synchronize the method every time

…itialisation in case of multithread environment.
@coveralls
Copy link

Coverage Status

Coverage increased (+0.003%) to 38.275% when pulling e888436 on bugfix/synchronisation-language-detection into d6ac4df on master.

@lfoppiano lfoppiano modified the milestones: 0.6.1, 0.6.0 May 26, 2020
@de-code
Copy link
Collaborator

de-code commented May 28, 2020

But whether it is worth the time saving over just synchronized I don't know. How often will it be called?

Looks to be the correct way of implementing double checked locking.

There are actually alternatives that you could consider as well:
Initialization on-demand holder (using the classloader)

I was previously in favour of using AtomicReference but it would just wrap the volatile. I am surprised that Java still doesn't seem to offer a simple lazy initialization wrapper where you can pass in a callable (like ThreadLocal).

Apache commons has LazyInitializer. Maybe that would be more readable.

@kermitt2
Copy link
Owner

Thank you Luca, it looks all good and solved the problem when testing with the grobid python client. Thank you @de-code for the pointers!
So, good to learn, lazyness requires more work :D

@kermitt2 kermitt2 merged commit 27ece93 into master May 30, 2020
@lfoppiano lfoppiano deleted the bugfix/synchronisation-language-detection branch May 31, 2020 21:24
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.

None yet

4 participants