Skip to content

[SYNCOPE-1635] Use singleton bean for each domain for each rule with conf - #264

Merged
ilgrosso merged 1 commit into
apache:2_1_Xfrom
DmitriyBrashevets:SYNCOPE-1635
May 21, 2021
Merged

[SYNCOPE-1635] Use singleton bean for each domain for each rule with conf#264
ilgrosso merged 1 commit into
apache:2_1_Xfrom
DmitriyBrashevets:SYNCOPE-1635

Conversation

@DmitriyBrashevets

Copy link
Copy Markdown
Contributor

…conf

String domainableBeanNameWithConf = domain + clazz.getName();
DefaultListableBeanFactory beanFactory = ApplicationContextProvider.getBeanFactory();

if (beanFactory.containsSingleton(domainableBeanNameWithConf)) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The same exact if with statement is also performed in the synchronized block: why?

@DmitriyBrashevets DmitriyBrashevets May 20, 2021

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This is a double-check lock pattern. Thread takes monitor and executes the logic regarding initialization. Another thread waits until the the monitor is realeased. As soon as first thread releases monitor -> second thread proceeds its work and reaches the additional if clause. As the bean already exists (created by first thread) the second thread is not creating a bean again and not registers it as a singleton one more time.

Do you see any disadvantages it this?

@ilgrosso ilgrosso May 20, 2021

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I see, there is wide literature abut double-check lock pattern, and some is not very pleasant with it, especially for Java implementations.

What would be the disadvantage of simply removing the first if and leaving the whole logic in the synchronized block?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

IMHO: Adding of the additional if clause gives a small improvement in performance (because the acquiring of monitor is not requested).

Am I wrong?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I am not completely convinced, but let that be :-)

@DmitriyBrashevets
DmitriyBrashevets force-pushed the SYNCOPE-1635 branch 2 times, most recently from 633532c to fbcbfb0 Compare May 20, 2021 14:59
@ilgrosso
ilgrosso merged commit 3ca3354 into apache:2_1_X May 21, 2021
@DmitriyBrashevets
DmitriyBrashevets deleted the SYNCOPE-1635 branch May 21, 2021 07:50
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.

2 participants