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

Default order of <mvc:resources> should be prioritized [SPR-14871] #19437

Closed
spring-projects-issues opened this issue Nov 2, 2016 · 3 comments
Assignees
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) status: declined A suggestion or change that we don't feel we should currently apply type: enhancement A general enhancement

Comments

@spring-projects-issues
Copy link
Collaborator

Tomoyuki Ikeya opened SPR-14871 and commented

Default order of <mvc:resources> is Order.LOWEST_PRECEDENCE - 1. This means search of mappings for static resources is done after all other mappings are evaluated.
Without changing default order, it takes overheads and causes performance problem especially when an app has a lot of accesses to static resources or request mappings.

Of course, it may be solved to set order explicitly such as <mvc:resources ... order="-1" />, but the current default order often leads performance problem.

To avoid the mis-leading, I propose to change the default order to -1, or improve reference document to mention to order option.


Affects: 4.1.9, 4.2.8, 4.3.3

2 votes, 4 watchers

@spring-projects-issues
Copy link
Collaborator Author

Brian Clozel commented

Hi Tomoyuki Ikeya

If we apply the changes you're suggesting, here's what could happen.
For an application that configured:

  • a REST endpoint (or anything handled by a Controller method) mapped on "/hello"
  • handling of static resources on "/**"

All "/hello" requests will be mapped to the ResourceHttpRequestHandler, which itself will respond with an HTTP 404 status if that resource does not exist.

Could elaborate a bit more on the performance issues you're facing?
How many mappings does your application have? (those are usually listed when your configure logging at the INFO level for org.springframework.web.servlet)
Did you perform some benchmarks and found a particular hot spot?

Thanks

@spring-projects-issues
Copy link
Collaborator Author

Tomoyuki Ikeya commented

Actually I think so. Changing default order impacts users who use @RequestMapping rather than static resources.
The order should be tuned case-by-case by each developer.

As an answer for your question, we encountered the performance problem in an app,

  • that has about 1000 @RequestMapping.
  • whose views usually have about 100 static resources links in each page.
  • Cache of static resources is disabled in client side (for the performance test).

We loaded on the app with 20 TPS requests.
Then the CPU usage rates increased more than 80% and TPS went down to less than 1 TPS.
After we changed the order (actually we have not tried with mvc:resources but DefaultServletHttpRequestHandler as https://github.com/terasolunaorg/guideline/issues/2348),
the TPS was improved to 20 TPS.

Actually the app was migrated from other framework such as Apache Struts1. Pre-existing app performed more than 20%.
As a result, I suspect that the matching process of @RequestMapping costs much more than that of mvc:resources or Struts1.

Note that we understand the app had better use CDN for its static resources to improve the performance.
But it was too difficult for the migration project to change the architecture drastically because of costs and time limitations.

@spring-projects-issues
Copy link
Collaborator Author

spring-projects-issues commented Dec 1, 2016

Brian Clozel commented

Thanks for all the details. So we both agree that we can't really change the default.

Indeed, the path matching process is done several times per requests and having 1000 mappings with potentially complex ones (with regexes?) is definitely going to take a toll on the performance. We've done multiple optimizations (see caches in AntPathMatcher) and we can always improve the situation if you can point us to a specific hot spot.

We're aware of that limitation and we're working on an alternate implementation of the PathMatcher which would be much more efficient (see #19112). This would be the default in the reactive web variant but should be an opt-in choice for MVC as the behavior might change slightly.

Thanks for this report!

@spring-projects-issues spring-projects-issues added status: declined A suggestion or change that we don't feel we should currently apply type: enhancement A general enhancement in: web Issues in web modules (web, webmvc, webflux, websocket) labels Jan 11, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) status: declined A suggestion or change that we don't feel we should currently apply type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

2 participants