Skip to content

Use ArgumentOutOfRangeException Throw methods in SendFileResponseExtensions..CheckRange #56420

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

Merged
merged 3 commits into from
Jul 15, 2024

Conversation

paulomorgado
Copy link
Contributor

To allow inlining by the JIT.

Use ArgumentOutOfRangeException Throw methods in SendFileResponseExtensions..CheckRange to allow inlining by the JIT

  • You've read the Contributor Guide and Code of Conduct.
  • You've included unit or integration tests for your change, where applicable.
  • You've included inline docs for your change, where applicable.
  • There's an open issue for the PR that you are making. If you'd like to propose a new feature or change, please open an issue to discuss the change or find an existing issue.

Summary of the changes (Less than 80 chars)

Description

Methods that throw exceptions are not inlinable by the JIT.

This PR uses the recently introduced throw methods in the ArgumentOutOfRangeException to allow the logic to be fully inlined in non-error situations to increase performance of static file responses.

…nsions..CheckRange

To allow inlining by the JIT.
@ghost ghost added the area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions label Jun 24, 2024
@dotnet-policy-service dotnet-policy-service bot added the community-contribution Indicates that the PR has been added by a community member label Jun 24, 2024
Co-authored-by: Andrew Casey <amcasey@users.noreply.github.com>
@amcasey amcasey enabled auto-merge (squash) June 27, 2024 18:56
@amcasey
Copy link
Member

amcasey commented Jun 27, 2024

FYI @captainsafia, I've enable auto-merge, since it seems straightforward. You have two hours to shout if you disagree. 😉

@paulomorgado
Copy link
Contributor Author

@captainsafia, @amcasey, looks like the build still has issues.

@amcasey
Copy link
Member

amcasey commented Jun 28, 2024

/azp run

Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@amcasey
Copy link
Member

amcasey commented Jun 28, 2024

@paulomorgado When that happens, you should be able to unblock yourself by closing and re-opening the PR. (We're happy to help, but you don't have to wait for us.) Obviously, we're working on making it happen less often.

auto-merge was automatically disabled June 28, 2024 17:21

Pull request was closed

@paulomorgado paulomorgado reopened this Jun 28, 2024
@dotnet-policy-service dotnet-policy-service bot added this to the 9.0-preview7 milestone Jun 28, 2024
@amcasey
Copy link
Member

amcasey commented Jun 28, 2024

But you don't need to do it after we manually restart the tests with /azp run. 😛

@dotnet-policy-service dotnet-policy-service bot added the pending-ci-rerun When assigned to a PR indicates that the CI checks should be rerun label Jul 6, 2024
@amcasey
Copy link
Member

amcasey commented Jul 15, 2024

Ack, apparently close and re-open disables auto-merge. TIL. Sorry about that.

@amcasey amcasey merged commit 8d9db2f into dotnet:main Jul 15, 2024
27 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions community-contribution Indicates that the PR has been added by a community member pending-ci-rerun When assigned to a PR indicates that the CI checks should be rerun
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants