[
https://issues.apache.org/jira/browse/WW-3461?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13049144#comment-13049144
]
István Székely commented on WW-3461:
------------------------------------
Only looking at the code and not taking any tests, the second patch which was
committed in r1078465 is wrong.
Consider the following scenario:
1. Thread A and Thread B are working parallel. They hit line 79
(validatorCache.containsKey(validatorKey)) at the same time.
2. Since validatorCache is a synchronized map either of the two threads has to
wait. This is the only guarantee that we have.
3. Both threads invoke validatorCache.containsKey(validatorKey), one after the
other, and get the result of false.
4. A and B both continue on line 84, validatorCache.put(...).
5. Again, one of the threads has to wait until the other one has finished BUT
nothing prevents that the buildValidatorConfigs(...) call in the argument list
be invoked parallel! We should investigate more what the
buildValidatorConfigs(...) can lock. Also, it can have performance drawbacks,
duplicate object creation and so on.
All of the containsKey(), put(), and get() method calls on the validatorCache
map should be performed as one atomic operation. The synchronized map can NOT
guarantee this. That's why you need a synchronized (validatorCache) { ... }
block at least.
> Don't hold locks as long when creating validators in
> AnnotationActionValidatorManager
> -------------------------------------------------------------------------------------
>
> Key: WW-3461
> URL: https://issues.apache.org/jira/browse/WW-3461
> Project: Struts 2
> Issue Type: Improvement
> Affects Versions: 2.1.6
> Reporter: Leigh Anderson
> Assignee: Lukasz Lenart
> Fix For: 2.2.3
>
> Attachments: WW-3461.patch, validatormanager.patch
>
>
> Under load, the lock held in AnnotationActionValidatorManager.getValidators
> on validatorCache becomes contended. This lock is currently held while all
> the validators are created. The attached patch reduces the scope of the lock
> to just doing the cache lookup + taking a defensive copy of the list of
> validators for that key. This improved throughput and latency in our tests.
--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira
