Author: etnu
Date: Sat Oct 18 14:15:17 2008
New Revision: 705924
URL: http://svn.apache.org/viewvc?rev=705924&view=rev
Log:
Switched to using a ThreadLocal instead of a builder pool when threading is
available. The pool was originally thought to be a better option, but in
practice the total number of threads in a container remains fairly constant due
to the use of thread pools on both executors and servlet containers.
The need for the reusability check may no longer be relevant since we now have
a hard dependency on xerces 2.9, which supports reusable builders.
Modified:
incubator/shindig/trunk/java/common/src/main/java/org/apache/shindig/common/xml/XmlUtil.java
Modified:
incubator/shindig/trunk/java/common/src/main/java/org/apache/shindig/common/xml/XmlUtil.java
URL:
http://svn.apache.org/viewvc/incubator/shindig/trunk/java/common/src/main/java/org/apache/shindig/common/xml/XmlUtil.java?rev=705924&r1=705923&r2=705924&view=diff
==============================================================================
---
incubator/shindig/trunk/java/common/src/main/java/org/apache/shindig/common/xml/XmlUtil.java
(original)
+++
incubator/shindig/trunk/java/common/src/main/java/org/apache/shindig/common/xml/XmlUtil.java
Sat Oct 18 14:15:17 2008
@@ -19,8 +19,6 @@
import org.apache.shindig.common.uri.Uri;
-import com.google.common.collect.Lists;
-
import org.w3c.dom.Element;
import org.w3c.dom.NamedNodeMap;
import org.w3c.dom.Node;
@@ -31,7 +29,6 @@
import java.io.IOException;
import java.io.StringReader;
-import java.util.List;
import java.util.logging.Level;
import java.util.logging.Logger;
@@ -60,11 +57,24 @@
}
};
- private static final List<DocumentBuilder> builderPool =
Lists.newArrayList();
- private static boolean canUsePooling = false;
+ private static boolean canReuseBuilders = false;
private static final DocumentBuilderFactory builderFactory
= DocumentBuilderFactory.newInstance();
+
+ private static final ThreadLocal<DocumentBuilder> reusableBuilder
+ = new ThreadLocal<DocumentBuilder>() {
+ @Override
+ protected DocumentBuilder initialValue() {
+ try {
+ LOG.info("Created a new document builder");
+ return builderFactory.newDocumentBuilder();
+ } catch (ParserConfigurationException e) {
+ throw new RuntimeException(e);
+ }
+ }
+ };
+
static {
// Disable various insecure and/or expensive options.
builderFactory.setValidating(false);
@@ -76,6 +86,7 @@
"http://xml.org/sax/features/external-general-entities";, false);
} catch (IllegalArgumentException e) {
// Not supported by some very old parsers.
+ LOG.info("XML parsers will load external general entities.");
}
try {
@@ -83,6 +94,7 @@
"http://xml.org/sax/features/external-parameter-entities";, false);
} catch (IllegalArgumentException e) {
// Not supported by some very old parsers.
+ LOG.info("XML parsers will load external parameter entities.");
}
try {
@@ -90,24 +102,29 @@
"http://apache.org/xml/features/nonvalidating/load-external-dtd";,
false);
} catch (IllegalArgumentException e) {
// Only supported by Apache's XML parsers.
+ LOG.info("XML parsers will load external DTDs.");
}
try {
builderFactory.setAttribute(XMLConstants.FEATURE_SECURE_PROCESSING,
true);
} catch (IllegalArgumentException e) {
// Not supported by older parsers.
+ LOG.info("Not using secure XML processing.");
}
try {
DocumentBuilder builder = builderFactory.newDocumentBuilder();
builder.reset();
- canUsePooling = true;
+ canReuseBuilders = true;
+ LOG.info("Reusing document builders");
} catch (UnsupportedOperationException e) {
// Only supported by newer parsers (xerces 2.8.x+ for instance).
- canUsePooling = false;
+ canReuseBuilders = false;
+ LOG.info("Not reusing document builders");
} catch (ParserConfigurationException e) {
// Only supported by newer parsers (xerces 2.8.x+ for instance).
- canUsePooling = false;
+ canReuseBuilders = false;
+ LOG.info("Not reusing document builders");
}
}
@@ -248,7 +265,7 @@
return getIntAttribute(node, attr, 0);
}
- /**
+ /**
* @return first child node matching the specified name
*/
public static Node getFirstNamedChildNode(Node root, String nodeName) {
@@ -265,18 +282,11 @@
/**
* Fetch a builder from the pool, creating a new one only if necessary.
*/
- private static DocumentBuilder getBuilderFromPool() throws
ParserConfigurationException {
+ private static DocumentBuilder getBuilder() throws
ParserConfigurationException {
DocumentBuilder builder;
- if (canUsePooling) {
- synchronized (builderPool) {
- int size = builderPool.size();
- if (size > 0) {
- builder = builderPool.remove(size - 1);
- builder.reset();
- } else {
- builder = builderFactory.newDocumentBuilder();
- }
- }
+ if (canReuseBuilders) {
+ builder = reusableBuilder.get();
+ builder.reset();
} else {
builder = builderFactory.newDocumentBuilder();
}
@@ -285,19 +295,6 @@
}
/**
- * Return a builder to the pool, allowing it to be re-used by future
operations.
- */
- private static void returnBuilderToPool(DocumentBuilder builder) {
- if (canUsePooling) {
- synchronized (builderPool) {
- builderPool.add(builder);
- }
- }
- }
-
-
-
- /**
* Attempts to parse the input xml into a single element.
* @param xml
* @return The document object
@@ -305,10 +302,9 @@
*/
public static Element parse(String xml) throws XmlException {
try {
- DocumentBuilder builder = getBuilderFromPool();
+ DocumentBuilder builder = getBuilder();
InputSource is = new InputSource(new StringReader(xml.trim()));
Element element = builder.parse(is).getDocumentElement();
- returnBuilderToPool(builder);
return element;
} catch (SAXParseException e) {
throw new XmlException(
