8176168: Performance drop due to SAXParser SymbolTable reset
Reviewed-by: joehw, lancea
--- 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";
-
}