Skip to content
This repository was archived by the owner on Mar 24, 2026. It is now read-only.

Round 13 - Yesod with Postgres#2247

Merged
NateBrady23 merged 20 commits intoTechEmpower:round-13from
vacationlabs:round-13
Aug 30, 2016
Merged

Round 13 - Yesod with Postgres#2247
NateBrady23 merged 20 commits intoTechEmpower:round-13from
vacationlabs:round-13

Conversation

@saurabhnanda
Copy link
Copy Markdown
Contributor

@saurabhnanda saurabhnanda commented Aug 29, 2016

Yesod with Postgres because the Haskell binding for MySQL has concurrency issues.

  • Cleaned up lots of code
  • Not dropping down to Wai/Warp level to micro-optimize for the benchmark
  • Using the most straight-forward/realistic/idiomatic Yesod code - straight out of the Yesod book (or tutorials)
  • The Fortunes benchmark is not working due to some encoding issues. Still investigating. Help would be appreciated.

@knewmanTE
Copy link
Copy Markdown
Contributor

@saurabhnanda thanks so much for the contribution! We're currently only taking bug fixes and minor changes for our round 13 test. All other merge requests should be opened against our round-14 branch. Would you mind closing this PR and re-opening one against our round-14 branch?

@saurabhnanda
Copy link
Copy Markdown
Contributor Author

@knewmanTE It would be great if you could include this in round 13 itself. I really want to make the Yesod benchmarks better, which seem to be suffering just because of the DB choice (MySQL vs Postgres).

All Travis CI tests are passing, so this shouldn't break anything. However, if you still feel that including this in round 13 won't be possible, I'll close and reopen against round 14.

*.o
*.sqlite3
.hsenv*
yesod-devel/
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This file should be merged with frameworks/Haskell/yesod-postgres/.gitignore.

Additionally, is there a reason to not simply add bench to .gitignore?

Copy link
Copy Markdown
Contributor Author

@saurabhnanda saurabhnanda Aug 29, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for looking into this. Any other changes that you think are required?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are welcome. No, I think the rest looks fine by me

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Made the change. Can't add bench to .gitignore because bench/src contains the source code.

@msmith-techempower
Copy link
Copy Markdown
Member

Looks good to me; if Travis-CI agrees, we'll merge it in.

@saurabhnanda
Copy link
Copy Markdown
Contributor Author

So, Travis CI passed. Just curious why did the yesod-postgres benchmark that I added, take approx 24 min whereas all the others are under 2 min?

@NateBrady23
Copy link
Copy Markdown
Member

@saurabhnanda We do some git diffing to try and limit the travis tests to only the frameworks that would be affected by the merge. Thanks for the contribution! Merging!

@NateBrady23 NateBrady23 merged commit 73443a3 into TechEmpower:round-13 Aug 30, 2016
@saurabhnanda
Copy link
Copy Markdown
Contributor Author

Thanks a lot! When will the preliminary data be updated? I'm eager to see how this benchmark performs.

@NateBrady23
Copy link
Copy Markdown
Member

@saurabhnanda I believe we kicked off another prelim round earlier today so this won't be in the next update. There may be one more just to make sure all the environments are working, but if not it will be in the final. After the new servers are up and we get this round done, our hope is the rounds will come much, much faster with an end goal being continuous benchmarking!

@LadyMozzarella LadyMozzarella added this to the Round 13 milestone Aug 30, 2016
@knewmanTE
Copy link
Copy Markdown
Contributor

I realize this is already merged in, but should yesod-postgres its own directory, or should this test have been added to the benchmark_config for the main yesod directory? ping @msmith-techempower @nbrady-techempower

@msmith-techempower
Copy link
Copy Markdown
Member

... yes, this should have been added to the existing yesod directory.

@nbrady-techempower Is on my list now

@NateBrady23
Copy link
Copy Markdown
Member

@msmith-techempower "Looks good to me; if Travis-CI agrees, we'll merge it in."

I'm follow my master blindly!

@msmith-techempower
Copy link
Copy Markdown
Member

msmith-techempower commented Aug 30, 2016

_<

Fine, you're off my list.

@saurabhnanda
Copy link
Copy Markdown
Contributor Author

Let me know what needs to be changed.
On 31 Aug 2016 1:51 am, "Mike Smith" notifications@github.com wrote:

_<

Fine, you're off my list.


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#2247 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AABu0fQ8Qp3lDnyfwS9KUqb9WTIj7T-dks5qlG8bgaJpZM4JvP_o
.

@NateBrady23
Copy link
Copy Markdown
Member

@saurabhnanda I hadn't noticed that this was a separate directory. Ideally, additional a tests for an existing framework would be contained within the same folder. So bringing this in to the existing yesod directory and adding the new test to the existing benchmark_config would be the proper approach.

@saurabhnanda
Copy link
Copy Markdown
Contributor Author

@nbrady-techempower Even within the yesod folder, there will have to be two completely separate directories for two separate stack projects - one called yesod-mysql and the other yesod-postgres. Is that acceptable?

Also, what is source_code used for? In this case what should it contain?

@saurabhnanda
Copy link
Copy Markdown
Contributor Author

I'm unable to run the yesod-mongodb-raw benchmarks. Did they ever work?

@saurabhnanda
Copy link
Copy Markdown
Contributor Author

Did you close round-13 to new PRs? I'm unable to raise a PR against round-13 from the following source: https://github.com/vacationlabs/FrameworkBenchmarks/tree/round-13

@msmith-techempower
Copy link
Copy Markdown
Member

  1. It is fine if you have yesod-mysql and yesod-postgres for the purposes of separating source code within the frameworks/Haskell/yesod folder.
  2. I have no idea if yesod-mongodb-raw ever worked but we (and many others) have been having issues with MongoDB lately, so I am not convinced as of yet that this is a problem.
  3. We closed the round-13 branch, but we are still accepting fixes for round-13. You should open new PRs against the master branch.

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.

5 participants