[master] OPENJPA-2767: Incomplete ValueMapDiscriminatorStrategy cache and Meta… #45
Conversation
…DataRepository race condition Signed-off-by: Will Dazey <dazeydev.3@gmail.com>
| */ | ||
| Class<?>[] processRegisteredClasses(ClassLoader envLoader) { | ||
| if (_registered.isEmpty()) | ||
| synchronized Class<?>[] processRegisteredClasses(ClassLoader envLoader) { |
There was a problem hiding this comment.
Any chance to avoid synchronization at method level?
There was a problem hiding this comment.
@ilgrosso I had written a possible implementation using read/write locks similar to my ValueMapDiscriminator impl, but so much of the code ended up being write locked that it almost seemed pointless to have that fine grained locking. Almost all the time, _registered is empty anyway, so little to no blocking will occur.
The issue really ends up being that all of processRegisteredClasses needs to block until it's done processing all of the classes out of the temporary array. I realize the temp array was added to reduce blocking on the _registered array in the first place, but that only opens a window for other threads to start initializing other caches WHILE the MetaDataRepository is still processing and building. If the ORM is sufficiently large (I was seeing this processing taking up to 700ms for my customer), it can stretch into application code as queries begin executing. During that time, reads are potentially dirty from MetaDataRepository._subs and we were seeing CNFE even though MetaDataRepository was STILL in the process of processing the class it threw a CNFE for.
That being said, if there is a strong performance concern from synchonizing the method (something that may not be as troubling nowadays with newer jdks), I can re-investigate implementing locks. This change was just infinitely cleaner and I have an interest for my customer to get this fixes as soon as possible.
There was a problem hiding this comment.
@dazey3 I see: given what you say, method synchronization could be acceptable then - I was just wondering why...
There was a problem hiding this comment.
@ilgrosso I understand the concern and I appreciate all the review comments I can get! Thank you for taking a look.
3 participants
https://issues.apache.org/jira/browse/OPENJPA-2767
Signed-off-by: Will Dazey dazeydev.3@gmail.com