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

Cache ServerHttpRequest::getMethod in AbstractServerHttpRequest #30139

Conversation

yuzawa-san
Copy link
Contributor

ServerHttpRequest.method() is called twice on each HttpMethodPredicate and within CorsUtils.isPreFlightRequest(). Each call has to do a roundtrip all the way into netty and it does a bunch of String hashing to resolve the proper Spring HttpMethod. I moved this work to be done once upon request construction since the field read is cheaper.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Mar 18, 2023
@sbrannen sbrannen added in: web Issues in web modules (web, webmvc, webflux, websocket) type: enhancement A general enhancement labels Mar 19, 2023
@poutsma poutsma self-assigned this Mar 20, 2023
@poutsma poutsma removed the status: waiting-for-triage An issue we've not yet triaged or decided on label Mar 20, 2023
@poutsma poutsma added this to the 6.0.8 milestone Mar 20, 2023
@poutsma
Copy link
Contributor

poutsma commented Mar 21, 2023

Thanks for leaving a PR.

It seems only fair that we introduce the same enhancement for the other ServerHttpRequest implementations, so I moved your changes to AbstractServerHttpRequest.

@poutsma poutsma changed the title Cache ServerHttpRequest.method() for Reactor Netty's Requests Cache ServerHttpRequest::method in AbstractServerHttpRequest Mar 21, 2023
@poutsma poutsma changed the title Cache ServerHttpRequest::method in AbstractServerHttpRequest Cache ServerHttpRequest::getMethod in AbstractServerHttpRequest Mar 21, 2023
@poutsma poutsma closed this in 37a4e84 Mar 21, 2023
poutsma added a commit that referenced this pull request Mar 21, 2023
* gh-30139:
  Cache ServerHttpRequest::getMethod in AbstractServerHttpRequest
  cache reactor request methods
mdeinum pushed a commit to mdeinum/spring-framework that referenced this pull request Jun 29, 2023
This commit ensures that the HttpMethod, exposed through
ServerHttpRequest::getMethod, is cached in AbstractServerHttpRequest so
that potentially expensive HTTP method lookups are only done once.

Closes spring-projectsgh-30139
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) type: enhancement A general enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants