Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace XMLReaderFactory with SAXParserFactory #27239

Closed

Conversation

Frederick888
Copy link
Contributor

@Frederick888 Frederick888 commented Aug 4, 2021

XMLReaderFactory has been marked as deprecated and without additional
configuration, and it's slower than SAXParserFactory.

Previously XMLReaderFactory.createXMLReader() is called upon every
request. This is an anti-pattern as mentioned in [1] and it can be very
slow since it loads the jar service file unless a parser has been
pre-assigned [2] (e.g. by setting org.xml.sax.driver).

SAXParserFactory uses a FactoryFinder [3] instead, which takes advantage
of a thread-local cache provided by ServiceLoader. Developers can still
pre-assign a factory by setting javax.xml.parsers.SAXParserFactory to
make it faster.

[1] https://bugs.openjdk.java.net/browse/JDK-6925410
[2] https://github.com/openjdk/jdk/blob/c8add223a10030e40ccef42e081fd0d8f00e0593/src/java.xml/share/classes/org/xml/sax/helpers/XMLReaderFactory.java#L144-L148
[3] https://github.com/openjdk/jdk/blob/66c653c561b3b5e904579af62e23ff94952bca05/src/java.xml/share/classes/javax/xml/parsers/SAXParserFactory.java#L181-L185


Related: #19055

Upstream discussion: https://mail.openjdk.java.net/pipermail/jdk-dev/2021-August/005853.html

XMLReaderFactory has been marked as deprecated and without additional
configuration, and it's slower than SAXParserFactory.

Previously `XMLReaderFactory.createXMLReader()` is called upon every
request. This is an anti-pattern as mentioned in [1] and it can be very
slow since it loads the jar service file unless a parser has been
pre-assigned [2] (e.g. by setting org.xml.sax.driver).

SAXParserFactory uses a FactoryFinder [3] instead, which takes advantage
of a thread-local cache provided by ServiceLoader. Developers can still
pre-assign a factory by setting javax.xml.parsers.SAXParserFactory to
make it faster.

[1] https://bugs.openjdk.java.net/browse/JDK-6925410
[2] https://github.com/openjdk/jdk/blob/c8add223a10030e40ccef42e081fd0d8f00e0593/src/java.xml/share/classes/org/xml/sax/helpers/XMLReaderFactory.java#L144-L148
[3] https://github.com/openjdk/jdk/blob/66c653c561b3b5e904579af62e23ff94952bca05/src/java.xml/share/classes/javax/xml/parsers/SAXParserFactory.java#L181-L185
@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Aug 4, 2021
@Frederick888 Frederick888 marked this pull request as ready for review August 4, 2021 08:00
@Frederick888
Copy link
Contributor Author

Frederick888 commented Aug 4, 2021

Sorry I should've replaced it outside spring-web as well.

UPDATE Pushed.

@sbrannen sbrannen added in: core Issues in core modules (aop, beans, core, context, expression) type: task A general task labels Aug 4, 2021
@snicoll snicoll added this to the Triage Queue milestone Nov 30, 2021
@snicoll
Copy link
Member

snicoll commented Nov 30, 2021

@Frederick888 thanks for the PR and sorry it took so long to process it.

With XMLReaderFactory being deprecated since 9 already and SAXParserFactory being the documented replacement, this looks good to me. With perhaps additional places in the meantime to check. WDYT @jhoeller?

@snicoll snicoll removed the status: waiting-for-triage An issue we've not yet triaged or decided on label Nov 30, 2021
@snicoll snicoll modified the milestones: Triage Queue, 6.0 M1 Nov 30, 2021
snicoll pushed a commit that referenced this pull request Dec 2, 2021
XMLReaderFactory has been marked as deprecated and without additional
configuration, and it's slower than SAXParserFactory.

Previously `XMLReaderFactory.createXMLReader()` is called upon every
request. This is an anti-pattern as mentioned in [1] and it can be very
slow since it loads the jar service file unless a parser has been
pre-assigned [2] (e.g. by setting org.xml.sax.driver).

SAXParserFactory uses a FactoryFinder [3] instead, which takes advantage
of a thread-local cache provided by ServiceLoader. Developers can still
pre-assign a factory by setting javax.xml.parsers.SAXParserFactory to
make it faster.

[1] https://bugs.openjdk.java.net/browse/JDK-6925410
[2] https://github.com/openjdk/jdk/blob/c8add223a10030e40ccef42e081fd0d8f00e0593/src/java.xml/share/classes/org/xml/sax/helpers/XMLReaderFactory.java#L144-L148
[3] https://github.com/openjdk/jdk/blob/66c653c561b3b5e904579af62e23ff94952bca05/src/java.xml/share/classes/javax/xml/parsers/SAXParserFactory.java#L181-L185

See gh-27239
snicoll added a commit that referenced this pull request Dec 2, 2021
@snicoll snicoll closed this in e0979d0 Dec 2, 2021
@snicoll snicoll self-assigned this Dec 2, 2021
@Frederick888 Frederick888 deleted the migrate-xml-reader branch January 9, 2023 04:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: core Issues in core modules (aop, beans, core, context, expression) type: task A general task
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants