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

Restructure Haskell/Warp for db modularity. #4595

Merged
merged 2 commits into from
Apr 1, 2019

Conversation

naushadh
Copy link
Contributor

@naushadh naushadh commented Apr 1, 2019

Was about to make some changes to Warp but quickly realized how cumbersome it was to re-apply patches by hand to each db. So decided to do some upfront one off refactoring to deduplicate db logic.

The database libs are now generic enough it can be trivially re-used with any server/framework. But that will require making tooling changes to make Docker run from parent dir so we can have say servant and warp share the reusable database libs. Will create a separate issue/PR for that.

Baseline stats from last PR (#4571):

. JSON Serialization Single Query Mutiple queries Fortunes Updates Plaintext
warp (65526da) 30,677 3,884 702 4,112 164 54,811
warp-hasql (65526da) 30,164 3,131 410 4,327 176 52,160
warp-mysql-haskell (65526da) 29,063 5,470 346 5,993 71 52,780

Current stats:

. JSON Serialization Single Query Mutiple queries Fortunes Updates Plaintext
warp 24,600 3,919 713 4,255 164 54,588
warp-hasql 25,787 4,865 410 4,623 173 53,855
warp-mysql-haskell 28,740 5,193 356 6,133 69 50,784

Looks mostly the same or better except for some odd regressions for JSON Serialization -- even that seems to get better on the later runs (warp-mysql-haskell vs warp) in the same test using the same code. I'm not too worried myself.

- Haskell/Warp dir has a lot of code duplication for testing various backend libraries as separate backend specific libraries.
- Refactored the project structure to move database specific modules into their own libraries so we can have a single/shared backend agnostic warp project. This makes it easier to add new backends with minimal effort.
This completes the code changes and docs following the earlier pure file moves (separate commit to preserve git history).

- Added `warp-shared` cabal file with mutliple executables using various backend drivers and the same shared server code.
- Updated stack.yaml to adopt the earlier directory layout changes. We now have backend specific libraries (`shared/tfb-*`) and a single re-usable server (`warp-shared`) lib.
- Updated docker file to build and install the new warp shared server -- the cabal file is responsible for producing backend specific executables that match `TFB_TEST_NAME`.
- Added ReadMes for each `shared/tfb-*` library.
- Updated `shared/tfb-types` to adopt to conform to uniform types that can play nice with all of the `shared/tfb-*` backends.
- Updated `shared/tfb-*` cabal files to adopt its new dir and concern of only being a backend driver.
- Updated `shared/tfb-*` `Db.hs` files to conform to the exact same public API so `warp-shared` can plug and play into any database driver without any custom code.
- Added ReadMe for `warp-shared` to document the executables/tests it produces.
- Updated `warp-shared` Lib to delegate connection pool management to `shared/tfb-*` backends as some use their own pools and others require use of `resource-pool` lib.
- Updated `warp-shared` Main to print test name so we can manually sanity check the right executable is running in the right TFB docker image.
@NateBrady23
Copy link
Member

Thanks @naushadh

As far as the context that the docker container pulls in, I'll see if I can add another benchmark_config option to pull in context from a directory above the framework test. I don't want do this automatically for all frameworks because I don't want to pull in the entire context of the language directory as it will make each test container larger. As it is, all these test containers take up a ton of disk space.

@NateBrady23 NateBrady23 merged commit 33a3d5b into TechEmpower:master Apr 1, 2019
@naushadh
Copy link
Contributor Author

naushadh commented Apr 5, 2019

See comment for performance results: #4571 (comment)

naushadh added a commit to naushadh/FrameworkBenchmarks that referenced this pull request Apr 7, 2019
- Starting an identical refactoring effort as done for Warp in: TechEmpower#4595
- For now we're just moving files we intend to re-use and purging the ones we will no longer need due to upcoming db modularity.
NateBrady23 pushed a commit that referenced this pull request Apr 8, 2019
* Restructure servant for db modularity.

- Starting an identical refactoring effort as done for Warp in: #4595
- For now we're just moving files we intend to re-use and purging the ones we will no longer need due to upcoming db modularity.

* Copy warp/shared into servant/shared.

Due to a docker restriction that disallows referencing shared code directories from a parent dir, and lack of tooling support to alter the scope dir of docker build invoke, we are unfortunately resorting to copy pasting code between frameworks. Documented this in the servant-shared README.

* Complete restructuring servant for code re-use.

- Updated benchmark config to reference updated docker file path.
- Updated benchmark config to add `postgres-wire` variation as we now support it.
- Updated stack.yaml to adopt the earlier directory layout changes. We now have backend specific libraries (`shared/tfb-*`) and a single re-usable server (`servant-shared`) executable.
- Updated cabal file with mutliple executables using various backend drivers and the same shared server code.
- Updated docker file to build and install the new `servant-shared` server -- the cabal file is responsible for producing backend specific executables that match `TFB_TEST_NAME`.
- Updated `servant-shared` Lib to delegate database queries to `shared/tfb-*` backends so we can keep the server code backend agnostic.
- Added MIME module to store all MIME things. Separated it out to keep the main Lib module focusing on pure server and response logic.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants