8176168: Performance drop due to SAXParser SymbolTable reset

Reviewed-by: joehw, lancea

jaxp/src/java.xml/share/classes/com/sun/org/apache/xerces/internal/parsers/XML11Configuration.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2008, 2016, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2008, 2017, Oracle and/or its affiliates. All rights reserved.
  */
 /*
  * Licensed to the Apache Software Foundation (ASF) under one or more
@@ -506,7 +506,8 @@ public class XML11Configuration extends ParserConfigurationSettings
             EXTERNAL_PARAMETER_ENTITIES,
             PARSER_SETTINGS,
             XMLConstants.FEATURE_SECURE_PROCESSING,
-            XMLConstants.USE_CATALOG
+            XMLConstants.USE_CATALOG,
+            JdkXmlUtils.RESET_SYMBOL_TABLE
         };
         addRecognizedFeatures(recognizedFeatures);
         // set state for default features
@@ -532,6 +533,7 @@ public class XML11Configuration extends ParserConfigurationSettings
         fFeatures.put(PARSER_SETTINGS, Boolean.TRUE);
         fFeatures.put(XMLConstants.FEATURE_SECURE_PROCESSING, Boolean.TRUE);
         fFeatures.put(XMLConstants.USE_CATALOG, JdkXmlUtils.USE_CATALOG_DEFAULT);
+        fFeatures.put(JdkXmlUtils.RESET_SYMBOL_TABLE, JdkXmlUtils.RESET_SYMBOL_TABLE_DEFAULT);
 
         // add default recognized properties
         final String[] recognizedProperties =
@@ -1585,11 +1587,12 @@ public class XML11Configuration extends ParserConfigurationSettings
 
 
     /**
-     * Reset the symbol table if it wasn't provided during construction
+     * Reset the symbol table if it wasn't provided during construction,
-     * and its not the first time when parse is called after initialization
+     * its not the first time when parse is called after initialization
+     * and RESET_SYMBOL_TABLE feature is set to true
      */
     private void resetSymbolTable() {
-        if (!fSymbolTableProvided) {
+        if (fFeatures.get(JdkXmlUtils.RESET_SYMBOL_TABLE) && !fSymbolTableProvided) {
             if (fSymbolTableJustInitialized) {
                 // Skip symbol table reallocation for the first parsing process
                 fSymbolTableJustInitialized = false;

jaxp/src/java.xml/share/classes/jdk/xml/internal/JdkXmlFeatures.java

@@ -27,6 +27,7 @@ package jdk.xml.internal;
 
 import javax.xml.XMLConstants;
 import static jdk.xml.internal.JdkXmlUtils.SP_USE_CATALOG;
+import static jdk.xml.internal.JdkXmlUtils.RESET_SYMBOL_TABLE;
 
 /**
  * This class manages JDK's XML Features. Previously added features and properties
@@ -61,7 +62,13 @@ public class JdkXmlFeatures {
          * The {@link javax.xml.XMLConstants.USE_CATALOG} feature.
          * FSP: USE_CATALOG is not enforced by FSP.
          */
-        USE_CATALOG(PROPERTY_USE_CATALOG, SP_USE_CATALOG, true, false, true, false);
+        USE_CATALOG(PROPERTY_USE_CATALOG, SP_USE_CATALOG, true, false, true, false),
+
+        /**
+         * Feature resetSymbolTable
+         * FSP: RESET_SYMBOL_TABLE_FEATURE is not enforced by FSP.
+         */
+        RESET_SYMBOL_TABLE_FEATURE(RESET_SYMBOL_TABLE, RESET_SYMBOL_TABLE, false, false, true, false);
 
         private final String name;
         private final String nameSP;

jaxp/src/java.xml/share/classes/jdk/xml/internal/JdkXmlUtils.java

@@ -51,6 +51,12 @@ public class JdkXmlUtils {
     public final static String CATALOG_PREFER = CatalogFeatures.Feature.PREFER.getPropertyName();
     public final static String CATALOG_RESOLVE = CatalogFeatures.Feature.RESOLVE.getPropertyName();
 
+    /**
+     * Reset SymbolTable feature
+     * System property name is identical to feature name
+     */
+    public final static String RESET_SYMBOL_TABLE = "jdk.xml.resetSymbolTable";
+
     /**
      * Values for a feature
      */
@@ -63,6 +69,13 @@ public class JdkXmlUtils {
     public static final boolean USE_CATALOG_DEFAULT
             = SecuritySupport.getJAXPSystemProperty(Boolean.class, SP_USE_CATALOG, "true");
 
+    /**
+     * Default value of RESET_SYMBOL_TABLE. This will read the System property
+     */
+    public static final boolean RESET_SYMBOL_TABLE_DEFAULT
+            = SecuritySupport.getJAXPSystemProperty(Boolean.class, RESET_SYMBOL_TABLE, "false");
+
+
     /**
      * JDK features (will be consolidated in the next major feature revamp
      */

jaxp/test/javax/xml/jaxp/unittest/sax/SymbolTableResetTest.java

@@ -23,6 +23,8 @@
 
 package sax;
 
+import static jaxp.library.JAXPTestUtilities.runWithAllPerm;
+
 import java.io.StringReader;
 
 import javax.xml.parsers.SAXParser;
@@ -36,43 +38,100 @@ import org.xml.sax.helpers.DefaultHandler;
 
 /*
  * @test
- * @bug 8173390
+ * @bug 8173390 8176168
  * @library /javax/xml/jaxp/libs /javax/xml/jaxp/unittest
- * @run testng/othervm -DrunSecMngr=true sax.SymbolTableResetTest
+ * @run testng/othervm -Djdk.xml.resetSymbolTable=false sax.SymbolTableResetTest
- * @run testng/othervm sax.SymbolTableResetTest
+ * @run testng/othervm -Djdk.xml.resetSymbolTable=true sax.SymbolTableResetTest
+ * @run testng/othervm -Djdk.xml.resetSymbolTable=false -DrunSecMngr=true sax.SymbolTableResetTest
+ * @run testng/othervm -Djdk.xml.resetSymbolTable=true -DrunSecMngr=true sax.SymbolTableResetTest
  * @summary Test that SAXParser reallocates symbol table during
  *          subsequent parse operations
  */
 @Listeners({jaxp.library.BasePolicy.class})
 public class SymbolTableResetTest {
 
+    /*
+     * Test verifies the following use cases when the parser feature is not set:
+     *  a) Reset symbol table is requested via the system property
+     *  b) Reset symbol table is not requested via the system property
+     *     and therefore the default value should be used - reset
+     *     operation should not occur.
+     */
+    @Test
+    public void testNoFeatureSet() throws Exception {
+        parseAndCheckReset(false, false);
+    }
+
+
+    /*
+     * Test that when symbol table reset is requested through parser
+     * feature it is not affected by the system property value
+     */
+    @Test
+    public void testResetEnabled() throws Exception {
+        parseAndCheckReset(true, true);
+    }
+
+    /*
+     * Test that when symbol table reset is disabled through parser
+     * feature it is not affected by the system property value
+     */
+    @Test
+    public void testResetDisabled() throws Exception {
+        parseAndCheckReset(true, false);
+    }
+
     /*
      * Test mimics the SAXParser usage in SAAJ-RI that reuses the
      * parsers from the internal pool. To avoid memory leaks, symbol
      * table associated with the parser should be reallocated during each
      * parse() operation.
      */
-    @Test
+    private void parseAndCheckReset(boolean setFeature, boolean value) throws Exception {
-    public void testReset() throws Exception {
+        // Expected result based on system property and feature
+        boolean resetExpected = setFeature && value;
+        // Indicates if system property is set
+        boolean spSet = runWithAllPerm(() -> System.getProperty(RESET_FEATURE)) != null;
         // Dummy xml input for parser
         String input = "<dummy>Test</dummy>";
-        // Create SAXParser
+
-        SAXParserFactory  spf = SAXParserFactory.newInstance();
+        // Check if system property is set only when feature setting is not requested
+        // and estimate if reset of symbol table is expected
+        if (!setFeature && spSet) {
+            resetExpected = runWithAllPerm(() -> Boolean.getBoolean(RESET_FEATURE));
+        }
+
+        // Create SAXParser and set feature if it is requested
+        SAXParserFactory spf = SAXParserFactory.newInstance();
+        if (setFeature) {
+            spf.setFeature(RESET_FEATURE, value);
+        }
         SAXParser p = spf.newSAXParser();
+
         // First parse iteration
         p.parse(new InputSource(new StringReader(input)), new DefaultHandler());
         // Get first symbol table reference
         Object symTable1 = p.getProperty(SYMBOL_TABLE_PROPERTY);
+
+        // reset parser
         p.reset();
+
         // Second parse iteration
         p.parse(new InputSource(new StringReader(input)), new DefaultHandler());
         // Get second symbol table reference
         Object symTable2 = p.getProperty(SYMBOL_TABLE_PROPERTY);
-        // Symbol table references should be different
+
-        Assert.assertNotSame(symTable1, symTable2, "Symbol table references");
+        // Check symbol table references after two subsequent parse operations
+        if (resetExpected) {
+            Assert.assertNotSame(symTable1, symTable2, "Symbol table references");
+        } else {
+            Assert.assertSame(symTable1, symTable2, "Symbol table references");
+        }
     }
 
+    // Reset symbol table feature
+    private static final String RESET_FEATURE = "jdk.xml.resetSymbolTable";
+
     // Symbol table property
     private static final String SYMBOL_TABLE_PROPERTY = "http://apache.org/xml/properties/internal/symbol-table";
-
 }