Multiple WstxInputFactory instances shouldn't use common SymbolTable instance

Hmmh. Are you sure your code is properly closing stream reader/writer instances? Factories should only get the shared instance once for bootstrapping purposes, and this should occur very infrequently. Also obviously factory instance are designed to be reused, which I assume is the case.

Yes, we are properly closing stream readers. We have a pool of factories that are reused for better performance and threads are blocking on a SymbolTable.makeChild() beacuse each factory has the same instance of SymbolTable (taken from final static SymbolTable mRootSymbols).

If each factory will have separate SymbolTable threads won't be blocking on it. I know it should happen infrequently, but in our case we have 16 CPUs on single box and few hunreds of threads reusing factories. In such case infrequently is very frequently .

Ok thank you for verifying this.

The way I remember code should work would not share root instance between multiple factories, so there might even be a bug in there. But I need to dig back in code to see for sure.. it has been quite a while since I touched it.

Hmmh. I don't think that the default behavior should be changed, since reuse and sharing of symbol tables is a big performance boost for most cases when dealing with small to medium-sized documents.
But perhaps a feature could be added to allow disabling this sharing

Alternatively I am definitely open to finding more efficient ways for handling locking: I still find it hard to believe that what should be a very short locking time would lead to significant contention without some kind of a flaw in implementation itself. And if so, implementation could be improved. I can not see any obvious problem however.

Ok, I agree that the default behavior should stay the same and separating SymbolTable`s could be an option. Then user could choose which option is the best for him, as he knows size of documents and what's more important for him: speed or memory footprint.

For me factories shouldn't have anything common as you can always use single instance if memory consumption matters. If memory is not the case each thread could have separate factory.

Regarding short locking time I wouldn't consider here time between locks. When you execute that code very often each time OS needs to do a context switch. In case of 16, 32, 64 and more CPUs it matters if all CPUs needs to clear cache and synchronize to execute few lines of code. So it's better to consume more memory for better performance.

Just to summarize, current default option would be used for small/embedded parsers and new option would be for multi-core servers.

Ok. Just out of curiosity, I will do one change that will reduce scope for lock time, and possibly reduce impact: this by moving actual construction out of sync section. I wish I could easily test if it has any measurable effect; but at least there should not be much downside.

On sharing: I am actually not as worried about memory usage, as that is mostly transient. Rather this is a performance optimization, as it avoids most String allocations and String.intern(), while still allowing String identity comparisons on keys; latter matters mostly with attribute names.

Regardless, I understand that with highest levels of concurrency different rules apply.
And I am actually excited to learn more, how to handle these cases, since they are bound to become more common. Also, this same problem with apply to Jackson JSON processor (another OSS project I authored).

I'll have to see what's the best way to avoid sync'ing altogether; and add a new configuration property as well.

Thank you for your help here!

Interesting. I wrote a test to see if improvements to synchronized blocks matter, and on my Mac (with dual multi-core CPU) I could get nice +50% throughput improvement when removing construction of child SymbolTable instance out of synchronized section.
I would be interested in knowing how this would change your set up: fix is in trunk as well as 4.1 branch. I will try to release 4.1.4 soon (for another fix).

Unfortunately, I won't be able to test your improvement. I changed project and I have different priorities now :/.

Ok. Well, thank you for your help; this should at least help others who are facing similar issues. And for what it is worth, it helped improve similar handling for Jackson JSON parser a lot as well.

Improved synchronization which should help with the problem.