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

Remove HEAD middleware #1170

Closed
xsawyerx opened this issue May 26, 2016 · 2 comments
Closed

Remove HEAD middleware #1170

xsawyerx opened this issue May 26, 2016 · 2 comments

Comments

@xsawyerx
Copy link
Member

Middlewares are expensive. The HEAD middleware can be removed because we're already at the point where we decide on the content or not in the core. There's no point to avoid a single if statement in order to add another wrapped subroutine.

Note the work I've done on optimizing Dancer2 - it involved removing several middlewares. This is another we don't need.

@xsawyerx
Copy link
Member Author

xsawyerx commented Sep 6, 2016

I tried implementing this quickly and as a reference, here are the benchmark results:

# Before (with Head Middleware)
8.5624e-01 +/- 6.8e-04 (0.1%)

# After (implementing it in core)
8.5342e-01 +/- 8.3e-04 (0.1%)

Doesn't seem big.

However, if I completely remove the FixMissingBodyInRedirect middleware (just to see how heavy it is), combined with the reimplementation of the Head middleware, we get:

8.2836e-01 +/- 3.0e-04 (0.0%)

Which is now approaching interesting. Not interesting enough though, because we would still have to implement that middleware, which doesn't seem trivial.

In short, for now I don't see a lot of value in removing either middleware.

@xsawyerx
Copy link
Member Author

Closing it for the reasoning mentioned above. Not much value in this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant