8176168: Performance drop due to SAXParser SymbolTable reset
authoraefimov
Mon, 24 Apr 2017 00:22:11 +0300
changeset 44802 89a545213cad
parent 44801 f4351089a6a0
child 44803 7765a2bc8e52
child 44872 6fca2d76380d
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
jaxp/src/java.xml/share/classes/jdk/xml/internal/JdkXmlFeatures.java
jaxp/src/java.xml/share/classes/jdk/xml/internal/JdkXmlUtils.java
jaxp/test/javax/xml/jaxp/unittest/sax/SymbolTableResetTest.java
--- a/jaxp/src/java.xml/share/classes/com/sun/org/apache/xerces/internal/parsers/XML11Configuration.java	Fri Apr 21 03:34:05 2017 +0000
+++ b/jaxp/src/java.xml/share/classes/com/sun/org/apache/xerces/internal/parsers/XML11Configuration.java	Mon Apr 24 00:22:11 2017 +0300
@@ -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 @@
             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 @@
         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 @@
 
 
     /**
-     * Reset the symbol table if it wasn't provided during construction
-     * and its not the first time when parse is called after initialization
+     * Reset the symbol table if it wasn't provided during construction,
+     * 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;
--- a/jaxp/src/java.xml/share/classes/jdk/xml/internal/JdkXmlFeatures.java	Fri Apr 21 03:34:05 2017 +0000
+++ b/jaxp/src/java.xml/share/classes/jdk/xml/internal/JdkXmlFeatures.java	Mon Apr 24 00:22:11 2017 +0300
@@ -27,6 +27,7 @@
 
 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 @@
          * 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;
--- a/jaxp/src/java.xml/share/classes/jdk/xml/internal/JdkXmlUtils.java	Fri Apr 21 03:34:05 2017 +0000
+++ b/jaxp/src/java.xml/share/classes/jdk/xml/internal/JdkXmlUtils.java	Mon Apr 24 00:22:11 2017 +0300
@@ -52,6 +52,12 @@
     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
      */
     public static final String FEATURE_TRUE = "true";
@@ -64,6 +70,13 @@
             = 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
      */
     public final static String CDATA_CHUNK_SIZE = "jdk.xml.cdataChunkSize";
--- a/jaxp/test/javax/xml/jaxp/unittest/sax/SymbolTableResetTest.java	Fri Apr 21 03:34:05 2017 +0000
+++ b/jaxp/test/javax/xml/jaxp/unittest/sax/SymbolTableResetTest.java	Mon Apr 24 00:22:11 2017 +0300
@@ -23,6 +23,8 @@
 
 package sax;
 
+import static jaxp.library.JAXPTestUtilities.runWithAllPerm;
+
 import java.io.StringReader;
 
 import javax.xml.parsers.SAXParser;
@@ -36,10 +38,12 @@
 
 /*
  * @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 sax.SymbolTableResetTest
+ * @run testng/othervm -Djdk.xml.resetSymbolTable=false 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
  */
@@ -47,32 +51,87 @@
 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
-    public void testReset() throws Exception {
+    private void parseAndCheckReset(boolean setFeature, boolean value) 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";
-
 }