Project

Profile

Help

Bug #3294

closed

Thread contention on LRUCache.get

Added by Fady Moussallam over 5 years ago. Updated about 5 years ago.

Status:
Closed
Priority:
Normal
Assignee:
Category:
Performance
Sprint/Milestone:
-
Start date:
2017-06-21
Due date:
% Done:

100%

Estimated time:
Legacy ID:
Applies to branch:
9.7, 9.8
Fix Committed on Branch:
9.7, 9.8
Fixed in Maintenance Release:
Platforms:

Description

Running XQuery on Saxon-PE 9.7.0-18 with 4 concurrent threads running the same query, I am seeing a lot of contentions between threads with the following stack trace:

pool-1-thread-3 [BLOCKED]

java.util.Collections$SynchronizedMap.get(Object)

net.sf.saxon.expr.sort.LRUCache.get(Object) LRUCache.java:58

net.sf.saxon.lib.ConversionRules.getConverter(AtomicType, AtomicType) ConversionRules.java:196

net.sf.saxon.expr.CastExpression.doCast(AtomicValue, XPathContext) CastExpression.java:366

net.sf.saxon.expr.CastExpression.evaluateItem(XPathContext) CastExpression.java:405

net.sf.saxon.expr.CastExpression.evaluateItem(XPathContext) CastExpression.java:31

net.sf.saxon.expr.GeneralComparison.effectiveBooleanValue(XPathContext) GeneralComparison.java:616

net.sf.saxon.expr.instruct.Choose.choose(XPathContext) Choose.java:865

net.sf.saxon.expr.instruct.Choose.evaluateItem(XPathContext) Choose.java:893

net.sf.saxon.expr.LetExpression.evaluateItem(XPathContext) LetExpression.java:521

net.sf.saxon.expr.parser.ExpressionTool.evaluate(Expression, int, XPathContext, int) ExpressionTool.java:314

net.sf.saxon.expr.LetExpression.eval(XPathContext) LetExpression.java:500

net.sf.saxon.expr.LetExpression.processLeavingTail(XPathContext) LetExpression.java:702

net.sf.saxon.expr.instruct.Block.processLeavingTail(XPathContext) Block.java:653

net.sf.saxon.expr.instruct.Instruction.process(XPathContext) Instruction.java:149

net.sf.saxon.expr.instruct.ElementCreator.processLeavingTail(XPathContext, NodeInfo) ElementCreator.java:364

net.sf.saxon.expr.instruct.ElementCreator.processLeavingTail(XPathContext) ElementCreator.java:311

net.sf.saxon.expr.instruct.Instruction.process(XPathContext) Instruction.java:149

net.sf.saxon.expr.flwor.ReturnClausePush.processTuple(XPathContext) ReturnClausePush.java:37

net.sf.saxon.expr.flwor.ForClausePush.processTuple(XPathContext) ForClausePush.java:44

net.sf.saxon.expr.flwor.FLWORExpression.process(XPathContext) FLWORExpression.java:864

net.sf.saxon.expr.instruct.UserFunction.process(Sequence[], XPathContextMajor) UserFunction.java:626

net.sf.saxon.expr.UserFunctionCall.process(XPathContext) UserFunctionCall.java:549

net.sf.saxon.expr.instruct.ElementCreator.processLeavingTail(XPathContext, NodeInfo) ElementCreator.java:364

net.sf.saxon.expr.instruct.ElementCreator.processLeavingTail(XPathContext) ElementCreator.java:311

net.sf.saxon.expr.instruct.Instruction.process(XPathContext) Instruction.java:149

net.sf.saxon.expr.flwor.ReturnClausePush.processTuple(XPathContext) ReturnClausePush.java:37

net.sf.saxon.expr.flwor.ForClausePush.processTuple(XPathContext) ForClausePush.java:44

net.sf.saxon.expr.flwor.FLWORExpression.process(XPathContext) FLWORExpression.java:864

net.sf.saxon.expr.instruct.UserFunction.process(Sequence[], XPathContextMajor) UserFunction.java:626

net.sf.saxon.expr.UserFunctionCall.process(XPathContext) UserFunctionCall.java:549

net.sf.saxon.query.XQueryExpression.run(DynamicQueryContext, Result, Properties) XQueryExpression.java:414

net.sf.saxon.s9api.XQueryEvaluator.run(Destination) XQueryEvaluator.java:370

org.talend.transform.xquery.SaxonProcessor.run(Source, ContentHandler) SaxonProcessor.java:48

org.talend.transform.engine.xquery.XQueryConcurTest$MyCallable.call() XQueryConcurTest.java:114

org.talend.transform.engine.xquery.XQueryConcurTest$MyCallable.call() XQueryConcurTest.java:1

java.lang.Thread.run()

forum link: https://saxonica.plan.io/boards/4/topics/6819

Attached the xquery we are running.

Thank you


Files

fcustdat.xquery (5.3 KB) fcustdat.xquery Fady Moussallam, 2017-06-21 13:34
Actions #1

Updated by Michael Kay over 5 years ago

See also the discussion a couple of years ago at https://saxonica.plan.io/boards/3/topics/6004,

Actions #2

Updated by Michael Kay over 5 years ago

Looking first at whether the query could be changed to avoid the problem:

Firstly, I think most of the casts are unnecessary. Consider the first let clause:

let $unconvertedValue := (
  if (fn:count($out_AVRO_root/AVRO:CUSTOMER-ID[1]) = 0) then ('_789odtMissing') else (
  if ($out_AVRO_root/AVRO:CUSTOMER-ID[1]/@xsi:nil['true']) then ('_osdt987Null') else ($out_AVRO_root/AVRO:CUSTOMER-ID[1]/text())  

we can see that on all branches the variable is bound either to a string or to a text node. Because it can be either, Saxon isn't going to be able to infer a very useful type for the variable, and therefore isn't going to be able to infer that a cast is unnecessary. But the effect of comparing $unconvertedValue to a string literal is exactly the same without the cast. I suspect that if these variables were declared (using "as xs:string") then most of the cast operations might be optimized away; alternatively, the cast operations could be explicitly removed. I haven't checked the whole query, and I don't fully understand the logic, but that's the way it appears at first glance.

Even if the cast expressions aren't optimized away, if Saxon can infer an explicit type for the operand of the cast, it will allocate the converter statically, avoiding run-time access to the cache that is acting as the contention spot.

Simplifying some of the expressions in the query would make it easier for Saxon to do this kind of type inferencing. For example

if (if (fn:count($unconvertedValue) = 1) then $unconvertedValue else $unconvertedValue[1]) cast as xs:string? = '_osdt987Null') then...

could be rewritten as

if ($unconvertedValue[1] cast as xs:string? = '_osdt987Null') then ...
Actions #3

Updated by Michael Kay over 5 years ago

Now looking at what we could do about it in Saxon. Here are some ideas, not necessarily mutually exclusive:

(A) replace the shared cache with a ThreadLocal cache. There's a lot of advice against using ThreadLocal, but we use it successfully for the cache of validated URIs.

(B) we currently special-case conversion FROM string to other types, bypassing the cache to allocate a StringConverter. We could do the same for conversion TO string, which is also a very common case.

(C) hold the cache in a ConcurrentHashMap, which is what we do for the TypeHierarchy cache. This wouldn't be an LRU cache; but if we can get away with an ever-growing cache for the TypeHierarchy, then we can probably get away with it here too.

(D) cache the converter from X to Y within the type object for Y, rather than globally.

(E) use statically-allocated converters for conversions between primitive types, and dynamically-allocated converters for non-primitive types, with no caching.

One complication is that the converter being allocated depends not only on the source and target type, but also on the ConversionRules in force. Handling of year zero and "+INF" depends on whether XSD 1.0 or 1.1 is in use, and handling of xs:NOTATION depends on the specific schema in use. This is why the converters are currently cached as part of the ConversionRules object rather than being held as a property of the source or target types.

Actions #4

Updated by Michael Kay over 5 years ago

  • Category set to Performance
  • Assignee set to Michael Kay

I have implemented what is essentially (B): the method ConversionRules.getConverter(), before it looks in the cache, handles some common cases directly:

        if (source == target) {
            return Converter.IDENTITY_CONVERTER;
        } else if (source == BuiltInAtomicType.STRING || source == BuiltInAtomicType.UNTYPED_ATOMIC) {
            return target.getStringConverter(this);
        } else if (target == BuiltInAtomicType.STRING) {
            return Converter.TO_STRING;
        } else if (target == BuiltInAtomicType.UNTYPED_ATOMIC) {
            return Converter.TO_UNTYPED_ATOMIC;
        }

Hopefully this will reduce the pressure on the cache; it certainly should do so for this particular query.

Actions #5

Updated by Michael Kay over 5 years ago

  • Status changed from New to Resolved
  • Priority changed from Low to Normal
  • Applies to branch 9.8 added
  • Fix Committed on Branch 9.7, 9.8 added

Patch committed on the 9.7 and 9.8 branches

Actions #6

Updated by O'Neil Delpratt over 5 years ago

  • % Done changed from 0 to 100
  • Fixed in Maintenance Release 9.8.0.2 added

Patch applied in the Saxon 9.8.0.2 maintenance release. Leaving bug issue marked as resolved until applied in the next Saxon 9.7 maintenance release.

Actions #7

Updated by O'Neil Delpratt about 5 years ago

  • Status changed from Resolved to Closed
  • Fixed in Maintenance Release 9.7.0.19 added

Bug fix applied in the 9.7.0.19 maintenance release.

Please register to edit this issue

Also available in: Atom PDF