[jdom-interest] Fast reconfiguration patch revision

[Scott Emmons]

Greetings Jdom group,

A couple of months ago I submitted an improvement (which made it into
1.1.1) to improve performance in cases where lots (in our cases
literally hundreds to thousands) of SAXBuilder.build() calls are made.
There is an issue with this patch under a circumstance where the fast
reconfig path won't happen. This is a "safe" bug in that it doesn't
functionally change anything, the faster codepath just isn't executed
(so it behaves as if setFastReconfigure() was not called).

The problem has to do with using a single boolean to guard both
lexical reporting configurations - which won't really work as intended
as there are three potential states here. In the patch below, this is
resolved in a much more universal and extensible way by having a map
for the features and a reusable method to handle the rest.

Note I tried to avoid more recent java language constructs and
features, but as written below it probably relies on autoboxing.

--- orig/src/java/org/jdom/input/SAXBuilder.java   2009-07-22
23:26:26.000000000 -0700
+++ src/java/org/jdom/input/SAXBuilder.java     2009-08-18
10:12:55.000000000 -0700
@@ -136,12 +136,9 @@

     /** Whether to use fast parser reconfiguration */
     private boolean fastReconfigure = false;
-
-    /** Whether to try lexical reporting in fast parser reconfiguration */
-    private boolean skipNextLexicalReportingConfig = false;
-
-    /** Whether to to try entity expansion in fast parser reconfiguration */
-    private boolean skipNextEntityExpandConfig = false;
+
+    /** Map for results of properties eligible for fast reconfiguration */
+    private HashMap skipNextConfig = new HashMap(3); //<String,Boolean>

     /**
      * Whether parser reuse is allowed.
@@ -691,68 +688,63 @@
              parser.setErrorHandler(new BuilderErrorHandler());
         }

-        // If fastReconfigure is enabled and we failed in the previous attempt
-        // in configuring lexical reporting, then we skip this step.  This
-        // saves the work of repeated exception handling on each parse.
-        if (!skipNextLexicalReportingConfig) {
-            boolean success = false;
-
-            try {
-
parser.setProperty("http://xml.org/sax/handlers/LexicalHandler",
-                                   contentHandler);
-                success = true;
-            } catch (SAXNotSupportedException e) {
-                // No lexical reporting available
-            } catch (SAXNotRecognizedException e) {
-                // No lexical reporting available
-            }
-
-            // Some parsers use alternate property for lexical
handling (grr...)
-            if (!success) {
-                try {
-
parser.setProperty("http://xml.org/sax/properties/lexical-handler",
-                                       contentHandler);
-                    success = true;
-                } catch (SAXNotSupportedException e) {
-                    // No lexical reporting available
-                } catch (SAXNotRecognizedException e) {
-                    // No lexical reporting available
-                }
-            }
-
-            // If unable to configure this property and fastReconfigure is
-            // enabled, then setup to avoid this code path entirely next time.
-            if (!success && fastReconfigure) {
-                skipNextLexicalReportingConfig = true;
-            }
+        // Set lexical reporting properties on parser, if this fails then try
+        // an alternate property that some parsers user
+        if (fastSetProperty(parser,
+                "http://xml.org/sax/handlers/LexicalHandler",
+                contentHandler) == false) {
+
+            fastSetProperty(parser,
+                    "http://xml.org/sax/properties/lexical-handler",
+                    contentHandler);
+        }
+
+        // Try setting the DeclHandler if entity expansion is off
+        if (!expand) {
+            fastSetProperty(parser,
+                    "http://xml.org/sax/properties/declaration-handler",
+                    contentHandler);
         }

-        // If fastReconfigure is enabled and we failed in the previous attempt
-        // in configuring entity expansion, then skip this step.  This
-        // saves the work of repeated exception handling on each parse.
-        if (!skipNextEntityExpandConfig) {
-            boolean success = false;
-
-            // Try setting the DeclHandler if entity expansion is off
-            if (!expand) {
-                try {
-
parser.setProperty("http://xml.org/sax/properties/declaration-handler",
-                                       contentHandler);
-                    success = true;
-                } catch (SAXNotSupportedException e) {
-                    // No lexical reporting available
-                } catch (SAXNotRecognizedException e) {
-                    // No lexical reporting available
-                }
-            }
-
-            /* If unable to configure this property and fastReconfigure is
-             * enabled, then setup to avoid this code path entirely next time.
-             */
-            if (!success && fastReconfigure) {
-                skipNextEntityExpandConfig = true;
-            }
+    }
+
+
+    /**
+     * Fast set property. Attempts to set a property on the parser. If the
+     * property setting fails (due to an exception from the parser), then next
+     * time we attempt to configure this parser instance, we just skip this
+     * because we already know it will fail.
+     * @param parser Parser to configure
+     * @param property Property to set
+     * @param contentHandler ContentHandler to set
+     * @return true if the property was set, false if it was not
+     */
+    private boolean fastSetProperty(XMLReader parser, String
property, SAXHandler contentHandler) {
+        boolean success=false;
+        Boolean haveCached=null;
+        if (fastReconfigure) {
+            haveCached=(Boolean)skipNextConfig.get(property);
+        }
+
+        // If we already have cached that we should skip this, then
just skip it
+        if (fastReconfigure && haveCached!=null &&
haveCached.booleanValue()==true) {
+            return false;
+        }
+
+        try {
+            parser.setProperty(property, contentHandler);
+            success=true;
+        } catch (SAXNotSupportedException e) {
+            // Property not available
+        } catch (SAXNotRecognizedException e) {
+            // Property not available
+        }
+
+        // If we've never seen this property before, then
+        if (fastReconfigure && haveCached==null) {
+            skipNextConfig.put(property, new Boolean(!success));
         }
+        return success;
     }

     private void setFeaturesAndProperties(XMLReader parser,


-- 
Scott Emmons