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

Possible optimization in UrlResource #25531

Closed
dsyer opened this issue Aug 5, 2020 · 3 comments
Closed

Possible optimization in UrlResource #25531

dsyer opened this issue Aug 5, 2020 · 3 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: enhancement A general enhancement
Milestone

Comments

@dsyer
Copy link
Member

dsyer commented Aug 5, 2020

UrlResource computes a cleanedUrl in its constructor, but only uses it in a small number of methods (e.g. hashCode()). It has some String manipulations in it that can be expensive, so it might make sense to compute it lazily.

Some benchmarks (instantiate a FileUrlResource and check if it exists()):

class                                method   median      mean        range
com.example.bench.ResourceBenchmark  lazy     441889.554  460384.372  16671.778
com.example.bench.ResourceBenchmark  default  415765.658  440910.870  18462.893

I think the fact that the median comparison is much more dramatic is probably because of garbage collection, which is quite noisy, and would be more common in the default case.

Spring Boot computes quite a few of these FileUrlResources on startup as it searches for application.properties files. The difference is just about noticeable (a few milliseconds on startup, and fewer GC failures).

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Aug 5, 2020
@jhoeller jhoeller self-assigned this Aug 5, 2020
@jhoeller jhoeller added in: core Issues in core modules (aop, beans, core, context, expression) type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Aug 5, 2020
@jhoeller jhoeller modified the milestones: 5.3 M2, 5.2.9 Aug 5, 2020
@jhoeller
Copy link
Contributor

jhoeller commented Aug 5, 2020

This turns out to be straightforward so that it can even be considered for 5.2.9.

One exception: UrlResource(String) needs the original input for the cleaned URL, so still builds it in the constructor where that original path is available. All the others, including the variants used by FileUrlResource, lazily resolve the cleaned URL now.

@dsyer
Copy link
Member Author

dsyer commented Aug 5, 2020

Great. In my tests I stored the original input in a field as well. That way it could always be lazy.

@jhoeller
Copy link
Contributor

jhoeller commented Aug 5, 2020

Indeed, but at the expense of an additional field sticking around for any kind of UrlResource usage... I'd rather compute it early then. The plain String constructor is generally not efficient in that respect. If String computations are to be avoided, the input should always be a URL or URI or at least a decomposed protocol plus location part.

@jhoeller jhoeller added the for: backport-to-5.1.x Marks an issue as a candidate for backport to 5.1.x label Aug 6, 2020
@spring-projects-issues spring-projects-issues added status: backported An issue that has been backported to maintenance branches and removed for: backport-to-5.1.x Marks an issue as a candidate for backport to 5.1.x labels Aug 6, 2020
jhoeller added a commit that referenced this issue Aug 7, 2020
jhoeller added a commit that referenced this issue Aug 7, 2020
FelixFly pushed a commit to FelixFly/spring-framework that referenced this issue Aug 16, 2020
zx20110729 pushed a commit to zx20110729/spring-framework that referenced this issue Feb 18, 2022
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: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

3 participants