`Play.XML` should use ThreadLocal SAXParser

[jilen]

Apache xerces uses a lot of Class.forName to create SaxParser object, which will call ClassLoader.loadClass for every xml parsing.

And this will block each other, then leads to bad performance for a high load system.

[gmethvin]

We removed xerces in #6342, which may fix the performance issues here.

It could probably be backported; I'm guessing the only issue would be if someone is using the xerces transitive dependency directly.

[jilen]

@gmethvin Unfortunately, jdk bundled xerces has the same issue. Just as this link I mentioned above described.

[schmitch]

wouldn't it make more sense to just have a final static DocumentBuilderFactory? Since than it would only load the classes once.

On the Scala version we already do it like that:

  private[play] val xercesSaxParserFactory = SAXParserFactory.newInstance()
  xercesSaxParserFactory.setFeature(Constants.SAX_FEATURE_PREFIX + Constants.EXTERNAL_GENERAL_ENTITIES_FEATURE, false)
  xercesSaxParserFactory.setFeature(Constants.SAX_FEATURE_PREFIX + Constants.EXTERNAL_PARAMETER_ENTITIES_FEATURE, false)
  xercesSaxParserFactory.setFeature(Constants.XERCES_FEATURE_PREFIX + Constants.DISALLOW_DOCTYPE_DECL_FEATURE, true)
  xercesSaxParserFactory.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true)

[jilen]

@schmitch @gmethvin You're rght. The master branch has no problem, but 2.5 and below needs a fix as I said, because apache xerces have lock while creating SaxParserImpl

"application-akka.actor.default-dispatcher-3" #16 prio=5 os_prio=0 tid=0x00007ffcb4bbd800 nid=0x661 runnable [0x00007ffc9c229000]
   java.lang.Thread.State: RUNNABLE
	at java.lang.ClassLoader.findLoadedClass0(Native Method)
	at java.lang.ClassLoader.findLoadedClass(ClassLoader.java:1038)
	at java.lang.ClassLoader.loadClass(ClassLoader.java:406)
	- locked <0x000000008034d000> (a java.lang.Object)
	at sun.misc.Launcher$AppClassLoader.loadClass(Launcher.java:331)
	at java.lang.ClassLoader.loadClass(ClassLoader.java:357)
	at org.apache.xerces.parsers.ObjectFactory.findProviderClass(Unknown Source)
	at org.apache.xerces.parsers.ObjectFactory.newInstance(Unknown Source)
	at org.apache.xerces.parsers.ObjectFactory.createObject(Unknown Source)
	at org.apache.xerces.parsers.ObjectFactory.createObject(Unknown Source)
	at org.apache.xerces.parsers.SAXParser.<init>(Unknown Source)
	at org.apache.xerces.parsers.SAXParser.<init>(Unknown Source)
	at org.apache.xerces.jaxp.SAXParserImpl$JAXPSAXParser.<init>(Unknown Source)
	at org.apache.xerces.jaxp.SAXParserImpl.<init>(Unknown Source)
	at org.apache.xerces.jaxp.SAXParserFactoryImpl.newSAXParser(Unknown Source)
	at play.api.Play$.XML(Play.scala:53)


"application-akka.actor.default-dispatcher-4" #17 prio=5 os_prio=0 tid=0x00007ffcb4bbe800 nid=0x662 waiting for monitor entry [0x00007ffc9c128000]
   java.lang.Thread.State: BLOCKED (on object monitor)
	at java.lang.ClassLoader.loadClass(ClassLoader.java:404)
	- waiting to lock <0x000000008034d000> (a java.lang.Object)
	at sun.misc.Launcher$AppClassLoader.loadClass(Launcher.java:331)
	at java.lang.ClassLoader.loadClass(ClassLoader.java:357)
	at org.apache.xerces.parsers.ObjectFactory.findProviderClass(Unknown Source)
	at org.apache.xerces.parsers.ObjectFactory.newInstance(Unknown Source)
	at org.apache.xerces.parsers.ObjectFactory.createObject(Unknown Source)
	at org.apache.xerces.parsers.ObjectFactory.createObject(Unknown Source)
	at org.apache.xerces.parsers.SAXParser.<init>(Unknown Source)
	at org.apache.xerces.parsers.SAXParser.<init>(Unknown Source)
	at org.apache.xerces.jaxp.SAXParserImpl$JAXPSAXParser.<init>(Unknown Source)
	at org.apache.xerces.jaxp.SAXParserImpl.<init>(Unknown Source)
	at org.apache.xerces.jaxp.SAXParserFactoryImpl.newSAXParser(Unknown Source)
	at play.api.Play$.XML(Play.scala:53)

[marcospereira]

Hey @jilen, thanks for reporting this.

Do you want to submit a pull request to fix this? #6342 would be a good start point. Keep in mind that, as @gmethvin said, we should not remove the xerces dependency for 2.5.x branch because people could be using it transitively.

Thanks.

[schmitch]

hm I wonder why that happens on 2.5.x since we also store the Factory inside a val:

https://github.com/playframework/playframework/blob/2.5.x/framework/src/play/src/main/scala/play/api/Play.scala#L44

maybe it would be enough to make it final val or lazy (lazy so that the compiler will generate a synchonized block)? Also I guess the code should be:

  private[play] lazy val xercesSaxParserFactory =  {
val parser = SAXParserFactory.newInstance("org.apache.xerces.jaxp.SAXParserFactoryImpl", Play.getClass.getClassLoader)
  parser.setFeature(Constants.SAX_FEATURE_PREFIX + Constants.EXTERNAL_GENERAL_ENTITIES_FEATURE, false)
  parser.setFeature(Constants.SAX_FEATURE_PREFIX + Constants.EXTERNAL_PARAMETER_ENTITIES_FEATURE, false)
  parser.setFeature(Constants.XERCES_FEATURE_PREFIX + Constants.DISALLOW_DOCTYPE_DECL_FEATURE, true)
parser.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true)
parser
}

and the java version should be updated.

[jilen]

@schmitch It is not xercesSaxParserFactory, but its method newSaxParser will also call Class.forName.

[jilen]

@marcospereira I will give a try

[gmethvin]

I think this is still an issue if our XML API doesn't perform well under load. We should either remove the API and leave it up to the user (like we did with yaml), or create something flexible enough to work for most people.