Skip to content
This repository has been archived by the owner on Feb 24, 2024. It is now read-only.

Improve performance of WrapBuffaloHandler #2146

Merged
merged 4 commits into from
Nov 20, 2021
Merged

Improve performance of WrapBuffaloHandler #2146

merged 4 commits into from
Nov 20, 2021

Conversation

fasmat
Copy link
Member

@fasmat fasmat commented Sep 11, 2021

This PR adds performance improvements to the WrapBuffaloHandler function.

In one of my apps I noticed that WrapBuffaloHandler is very slow for what it is trying to do. The reason is that every time the wrapped handler is called a new instance of a buffalo.App is created which is very slow and unnecessary.

This PR should fix the problem. The first commit only contains a benchmark and the commit(s) after that the changes, to be able to see the differences. On my machine I was able to achieve a ~ 40x performance increase with this change.

I also extended the documentation of the relevant function to inform the user that the functionality comes with some drawbacks.

@fasmat fasmat requested a review from a team as a code owner September 11, 2021 21:49
@fasmat fasmat changed the title Add Benchmark to test performance improvements Improve performance of WrapBuffaloHandler Sep 11, 2021
@github-actions
Copy link

This PR is stale because it has been open 45 days with no activity. Remove stale label or comment or this will be closed in 10 days.

@fasmat
Copy link
Member Author

fasmat commented Oct 27, 2021

Any chance this will get merged?

@fasmat
Copy link
Member Author

fasmat commented Nov 13, 2021

I simplified the code by merging the functionality of the wrappedContext (only used in wrapped handlers) into the default context.

@paganotoni paganotoni merged commit 4644ecc into gobuffalo:master Nov 20, 2021
@paganotoni
Copy link
Member

Thanks @fasmat and @stanislas-m !

@fasmat fasmat deleted the feature/performance-improvements-wrapbuffalohandler branch November 24, 2021 13:16
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants