Skip to content

Commit

Permalink
Issue #5095 - XmlConfiguration Parser Pool (#5148)
Browse files Browse the repository at this point in the history 
* Issue #5095 XmlConfiguration Parser Pool

Use a pool of parsers rather than a shared static

Signed-off-by: Greg Wilkins <[email protected]>

* Some updates to the new Pool class:

 + fixed a race with pending reservations
 + use a pending counter
 + Reservation API to simplify Entry API
 + removed public methods on Entry API

* Some updates to the new Pool class:

 + fixed a race with pending reservations
 + use a pending counter
 + Reservation API to simplify Entry API
 + removed public methods on Entry API

* Updates from review

* Updates from review
Tests for cache size and acquire with creator

* Method no longer required with Reservation

* update from the feedback on the feedback of the feedback from the review.

Moved enable to Entry, removed Reservation class and clarified usage in javadoc

* Issue #5095 XmlConfiguration locking  Use pool instead of static shared instance

* removed fake test

Signed-off-by: Greg Wilkins <[email protected]>

* Issue #5095 XmlConfiguration locking  Use pool instead of static shared instance

updates from review
  • Loading branch information
@gregw
gregw authored Aug 24, 2020
1 parent f143bab commit 1159fa3
Show file tree
Hide file tree
Showing 2 changed files with 57 additions and 49 deletions.
7 changes: 0 additions & 7 deletions jetty-util/src/test/java/org/eclipse/jetty/util/PoolTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,13 +49,6 @@ public static Stream<Object[]> cacheSize()
return data.stream();
}

@ParameterizedTest
@MethodSource(value = "cacheSize")
public void testCache(int cacheSize)
{
System.err.println(cacheSize);
}

@ParameterizedTest
@MethodSource(value = "cacheSize")
public void testAcquireRelease(int cacheSize)
Expand Down
99 changes: 57 additions & 42 deletions jetty-xml/src/main/java/org/eclipse/jetty/xml/XmlConfiguration.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@
import org.eclipse.jetty.util.LazyList;
import org.eclipse.jetty.util.Loader;
import org.eclipse.jetty.util.MultiException;
import org.eclipse.jetty.util.Pool;
import org.eclipse.jetty.util.StringUtil;
import org.eclipse.jetty.util.TypeUtil;
import org.eclipse.jetty.util.annotation.Name;
Expand Down Expand Up @@ -99,7 +100,8 @@ public class XmlConfiguration
ArrayList.class, HashSet.class, Queue.class, List.class, Set.class, Collection.class
};
private static final Iterable<ConfigurationProcessorFactory> __factoryLoader = ServiceLoader.load(ConfigurationProcessorFactory.class);
private static final XmlParser __parser = initParser();
private static final Pool<ConfigurationParser> __parsers =
new Pool<>(Math.min(8, Runtime.getRuntime().availableProcessors()),1);
public static final Comparator<Executable> EXECUTABLE_COMPARATOR = (e1, e2) ->
{
// Favour methods with less parameters
Expand Down Expand Up @@ -144,31 +146,6 @@ public class XmlConfiguration
return compare;
};

private static XmlParser initParser()
{
ClassLoader loader = XmlConfiguration.class.getClassLoader();
XmlParser parser = new XmlParser();
URL config60 = loader.getResource("org/eclipse/jetty/xml/configure_6_0.dtd");
URL config76 = loader.getResource("org/eclipse/jetty/xml/configure_7_6.dtd");
URL config90 = loader.getResource("org/eclipse/jetty/xml/configure_9_0.dtd");
URL config93 = loader.getResource("org/eclipse/jetty/xml/configure_9_3.dtd");
parser.redirectEntity("configure.dtd", config90);
parser.redirectEntity("configure_1_0.dtd", config60);
parser.redirectEntity("configure_1_1.dtd", config60);
parser.redirectEntity("configure_1_2.dtd", config60);
parser.redirectEntity("configure_1_3.dtd", config60);
parser.redirectEntity("configure_6_0.dtd", config60);
parser.redirectEntity("configure_7_6.dtd", config76);
parser.redirectEntity("configure_9_0.dtd", config90);
parser.redirectEntity("configure_9_3.dtd", config93);
parser.redirectEntity("http://jetty.mortbay.org/configure.dtd", config93);
parser.redirectEntity("http://jetty.eclipse.org/configure.dtd", config93);
parser.redirectEntity("http://www.eclipse.org/jetty/configure.dtd", config93);
parser.redirectEntity("-//Mort Bay Consulting//DTD Configure//EN", config93);
parser.redirectEntity("-//Jetty//Configure//EN", config93);
return parser;
}

/**
* Set the standard IDs and properties expected in a jetty XML file:
* <ul>
Expand Down Expand Up @@ -226,6 +203,14 @@ public static String normalizeURI(String uri)
private final String _dtd;
private ConfigurationProcessor _processor;

ConfigurationParser getParser()
{
Pool<ConfigurationParser>.Entry entry = __parsers.acquire(ConfigurationParser::new);
if (entry == null)
return new ConfigurationParser(null);
return entry.getPooled();
}

/**
* Reads and parses the XML configuration file.
*
Expand All @@ -235,14 +220,11 @@ public static String normalizeURI(String uri)
*/
public XmlConfiguration(Resource resource) throws SAXException, IOException
{
synchronized (__parser)
try (ConfigurationParser parser = getParser(); InputStream inputStream = resource.getInputStream())
{
_location = resource;
try (InputStream inputStream = resource.getInputStream())
{
setConfig(__parser.parse(inputStream));
}
_dtd = __parser.getDTD();
setConfig(parser.parse(inputStream));
_dtd = parser.getDTD();
}
}

Expand Down Expand Up @@ -275,15 +257,12 @@ public XmlConfiguration(String configuration) throws SAXException, IOException
configuration = "<?xml version=\"1.0\" encoding=\"utf-8\"?>\n" +
"<!DOCTYPE Configure PUBLIC \"-//Jetty//Configure//EN\" \"http://www.eclipse.org/jetty/configure_9_3.dtd\">" +
configuration;
try (StringReader reader = new StringReader(configuration))
try (ConfigurationParser parser = getParser(); StringReader reader = new StringReader(configuration))
{
InputSource source = new InputSource(reader);
synchronized (__parser)
{
_location = null;
setConfig(__parser.parse(source));
_dtd = __parser.getDTD();
}
_location = null;
setConfig(parser.parse(source));
_dtd = parser.getDTD();
}
}

Expand All @@ -299,11 +278,11 @@ public XmlConfiguration(String configuration) throws SAXException, IOException
public XmlConfiguration(InputStream configuration) throws SAXException, IOException
{
InputSource source = new InputSource(configuration);
synchronized (__parser)
try (ConfigurationParser parser = getParser())
{
_location = null;
setConfig(__parser.parse(source));
_dtd = __parser.getDTD();
setConfig(parser.parse(source));
_dtd = parser.getDTD();
}
}

Expand Down Expand Up @@ -1939,4 +1918,40 @@ else if (arg.toLowerCase(Locale.ENGLISH).endsWith(".properties"))
throw e;
}
}

private static class ConfigurationParser extends XmlParser implements AutoCloseable
{
private final Pool<ConfigurationParser>.Entry _entry;

private ConfigurationParser(Pool<ConfigurationParser>.Entry entry)
{
_entry = entry;
ClassLoader loader = XmlConfiguration.class.getClassLoader();
URL config60 = loader.getResource("org/eclipse/jetty/xml/configure_6_0.dtd");
URL config76 = loader.getResource("org/eclipse/jetty/xml/configure_7_6.dtd");
URL config90 = loader.getResource("org/eclipse/jetty/xml/configure_9_0.dtd");
URL config93 = loader.getResource("org/eclipse/jetty/xml/configure_9_3.dtd");
redirectEntity("configure.dtd", config90);
redirectEntity("configure_1_0.dtd", config60);
redirectEntity("configure_1_1.dtd", config60);
redirectEntity("configure_1_2.dtd", config60);
redirectEntity("configure_1_3.dtd", config60);
redirectEntity("configure_6_0.dtd", config60);
redirectEntity("configure_7_6.dtd", config76);
redirectEntity("configure_9_0.dtd", config90);
redirectEntity("configure_9_3.dtd", config93);
redirectEntity("http://jetty.mortbay.org/configure.dtd", config93);
redirectEntity("http://jetty.eclipse.org/configure.dtd", config93);
redirectEntity("http://www.eclipse.org/jetty/configure.dtd", config93);
redirectEntity("-//Mort Bay Consulting//DTD Configure//EN", config93);
redirectEntity("-//Jetty//Configure//EN", config93);
}

@Override
public void close()
{
if (_entry != null)
__parsers.release(_entry);
}
}
}

0 comments on commit 1159fa3

Please sign in to comment.