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

Performance impact of con.getContentLengthLong() in AbstractFileResolvingResource.isReadable() downloading huge jars to check component length #27541

Closed
manisha-shetty opened this issue Oct 10, 2021 · 6 comments
Assignees
Labels
in: core Issues in core modules (aop, beans, core, context, expression) status: backported An issue that has been backported to maintenance branches type: regression A bug that is also a regression
Milestone

Comments

@manisha-shetty
Copy link

manisha-shetty commented Oct 10, 2021

Affects: Spring Framework 5.2.9

Change causing the issue: 69f14a2

The above change added content length check to the AbstractFileResolvingResource.isReadable() method. That introduced performance impact on using webstart.

For URL connections over Internet (and not on file system), if the context-scan involves component scan for certain packages, and those packages are inside a jar then that blocks the thread.
Jar is downloaded as many times as there are components appearing in packages meant to be scanned in XML file.

EXAMPLE:

Consider this: <context:component-scan base-package="com.spring.test" />

Now if I have a 1gb jar (say, test.jar) with the above package in my application,
I have a jnlp file with <jar main="false" href="jars/test.jar"/>, and test.jar has lets say 50 classes in the package com.spring,test; then when starting the jnlp application for the first time, spring scans for components and as the jar is hosted over the internet (not available in cache for the first time) , spring downloads it 50 times and the thread is in runnable state for a long time thus not allowing other applications to connect to the server hosting the webstart application.

Can we make the con.getContentLengthLong() optional??

SAMPLE RUNNABLE THREAD:

"com.test.TestApp$2" #174 prio=5 os_prio=0 tid=0x0000000027a0f000 nid=0x38f4 runnable [0x000000002d0ec000]
   java.lang.Thread.State: RUNNABLE
	at java.net.SocketInputStream.socketRead0(Native Method)
	at java.net.SocketInputStream.socketRead(Unknown Source)
	at java.net.SocketInputStream.read(Unknown Source)
	at java.net.SocketInputStream.read(Unknown Source)
	at java.io.BufferedInputStream.fill(Unknown Source)
	at java.io.BufferedInputStream.read1(Unknown Source)
	at java.io.BufferedInputStream.read(Unknown Source)
	- locked <0x000000008b1da7b0> (a java.io.BufferedInputStream)
	at sun.net.www.http.HttpClient.parseHTTPHeader(Unknown Source)
	at sun.net.www.http.HttpClient.parseHTTP(Unknown Source)
	at sun.net.www.protocol.http.HttpURLConnection.getInputStream0(Unknown Source)
	- locked <0x000000008b1cc770> (a sun.net.www.protocol.http.HttpURLConnection)
	at sun.net.www.protocol.http.HttpURLConnection.getInputStream(Unknown Source)
	- locked <0x000000008b1cc770> (a sun.net.www.protocol.http.HttpURLConnection)
	at sun.net.www.protocol.http.HttpURLConnection.getHeaderField(Unknown Source)
	at java.net.URLConnection.getHeaderFieldLong(Unknown Source)
	at java.net.URLConnection.getContentLengthLong(Unknown Source)
	at sun.net.www.protocol.jar.JarURLConnection.getContentLengthLong(Unknown Source)
	at org.springframework.core.io.AbstractFileResolvingResource.isReadable(AbstractFileResolvingResource.java:109)
	at org.springframework.context.annotation.ClassPathScanningCandidateComponentProvider.scanCandidateComponents(ClassPathScanningCandidateComponentProvider.java:427)
	at org.springframework.context.annotation.ClassPathScanningCandidateComponentProvider.findCandidateComponents(ClassPathScanningCandidateComponentProvider.java:315)
	at org.springframework.context.annotation.ClassPathBeanDefinitionScanner.doScan(ClassPathBeanDefinitionScanner.java:276)
	at org.springframework.context.annotation.ComponentScanBeanDefinitionParser.parse(ComponentScanBeanDefinitionParser.java:90)
	at org.springframework.beans.factory.xml.NamespaceHandlerSupport.parse(NamespaceHandlerSupport.java:74)
	at org.springframework.beans.factory.xml.BeanDefinitionParserDelegate.parseCustomElement(BeanDefinitionParserDelegate.java:1391)
	at org.springframework.beans.factory.xml.BeanDefinitionParserDelegate.parseCustomElement(BeanDefinitionParserDelegate.java:1371)
	at org.springframework.beans.factory.xml.DefaultBeanDefinitionDocumentReader.parseBeanDefinitions(DefaultBeanDefinitionDocumentReader.java:179)
	at org.springframework.beans.factory.xml.DefaultBeanDefinitionDocumentReader.doRegisterBeanDefinitions(DefaultBeanDefinitionDocumentReader.java:149)
	at org.springframework.beans.factory.xml.DefaultBeanDefinitionDocumentReader.registerBeanDefinitions(DefaultBeanDefinitionDocumentReader.java:96)
	at org.springframework.beans.factory.xml.XmlBeanDefinitionReader.registerBeanDefinitions(XmlBeanDefinitionReader.java:511)
	at org.springframework.beans.factory.xml.XmlBeanDefinitionReader.doLoadBeanDefinitions(XmlBeanDefinitionReader.java:391)
	at org.springframework.beans.factory.xml.XmlBeanDefinitionReader.loadBeanDefinitions(XmlBeanDefinitionReader.java:338)
	at org.springframework.beans.factory.xml.XmlBeanDefinitionReader.loadBeanDefinitions(XmlBeanDefinitionReader.java:310)
	at org.springframework.beans.factory.support.AbstractBeanDefinitionReader.loadBeanDefinitions(AbstractBeanDefinitionReader.java:188)
	at org.springframework.beans.factory.support.AbstractBeanDefinitionReader.loadBeanDefinitions(AbstractBeanDefinitionReader.java:224)
	at org.springframework.beans.factory.support.AbstractBeanDefinitionReader.loadBeanDefinitions(AbstractBeanDefinitionReader.java:195)
	at org.springframework.beans.factory.support.AbstractBeanDefinitionReader.loadBeanDefinitions(AbstractBeanDefinitionReader.java:257)
	at org.springframework.context.support.AbstractXmlApplicationContext.loadBeanDefinitions(AbstractXmlApplicationContext.java:128)
	at org.springframework.context.support.AbstractXmlApplicationContext.loadBeanDefinitions(AbstractXmlApplicationContext.java:94)
	at org.springframework.context.support.AbstractRefreshableApplicationContext.refreshBeanFactory(AbstractRefreshableApplicationContext.java:130)
	at org.springframework.context.support.AbstractApplicationContext.obtainFreshBeanFactory(AbstractApplicationContext.java:638)
	at org.springframework.context.support.AbstractApplicationContext.refresh(AbstractApplicationContext.java:523)
	- locked <0x00000000a58dc9a8> (a java.lang.Object)
	at org.springframework.context.support.ClassPathXmlApplicationContext.<init>(ClassPathXmlApplicationContext.java:144)
	at org.springframework.context.support.ClassPathXmlApplicationContext.<init>(ClassPathXmlApplicationContext.java:109)
@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Oct 10, 2021
@jhoeller jhoeller added in: core Issues in core modules (aop, beans, core, context, expression) type: bug A general bug and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Oct 11, 2021
@jhoeller jhoeller self-assigned this Oct 11, 2021
@jhoeller jhoeller added this to the 5.3.11 milestone Oct 11, 2021
@jhoeller jhoeller added type: regression A bug that is also a regression and removed type: bug A general bug labels Oct 11, 2021
@jhoeller jhoeller changed the title Performance Impact due to con.getContentLengthLong() in AbstractFileResolvingResource.isReadable() downloading huge jars to check component length Performance impact of con.getContentLengthLong() in AbstractFileResolvingResource.isReadable() downloading huge jars to check component length Oct 12, 2021
@jhoeller
Copy link
Contributor

This originates in #21372 which led to a reimplementation of AbstractFileResolvingResource.isReadable() in 5.1. I'm about to address it along with #27541 for the 5.3.11 release, with the intention to backport it to 5.2.18 as well.

@jhoeller
Copy link
Contributor

On review, the resource.isReadable() call is effectively unnecessary there and can be replaced with straight getInputStream() access and catching of FileNotFoundException, same for entity scanning in our Hibernate/JPA builders. This avoids any extra overhead to begin with, simply trying to process the resource and backing out if necessary.

The 5.1 revision was meant for static web resources where those additional checks provide value for testing whether a specific resource path can be served. In the context of classpath scanning, the tradeoff is quite different since we process resources coming from a ResourcePatternResolver where we can optimistically assume that they exist and usually allow for reading.

@jhoeller
Copy link
Contributor

jhoeller commented Oct 12, 2021

Closed through 715f300 (unfortunately with a typo in the issue number in the commit message).

@manisha-shetty
Copy link
Author

manisha-shetty commented Oct 12, 2021

Thanks for the quick review and resolution @jhoeller .

Will you be backporting to 5.2.x? If yes could you please share the version it should be included in?

@jhoeller
Copy link
Contributor

I'm in the process of backporting it to 5.2.18, will be creating a backport issue on GitHub in a moment...

@jhoeller jhoeller added the for: backport-to-5.2.x Marks an issue as a candidate for backport to 5.2.x label Oct 12, 2021
@spring-projects-issues spring-projects-issues added status: backported An issue that has been backported to maintenance branches and removed for: backport-to-5.2.x Marks an issue as a candidate for backport to 5.2.x labels Oct 12, 2021
@jhoeller
Copy link
Contributor

This is available in the latest 5.3.11 snapshot now and will be available in the upcoming 5.2.18 snapshot as well. Feel free to give it an early try...

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) status: backported An issue that has been backported to maintenance branches type: regression A bug that is also a regression
Projects
None yet
Development

No branches or pull requests

3 participants