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

Realistic low-level approach with "rapidoid-http-fast"#2257

Merged
knewmanTE merged 5 commits intoTechEmpower:masterfrom
nmihajlovski:master
Sep 16, 2016
Merged

Realistic low-level approach with "rapidoid-http-fast"#2257
knewmanTE merged 5 commits intoTechEmpower:masterfrom
nmihajlovski:master

Conversation

@nmihajlovski
Copy link
Copy Markdown
Contributor

@nmihajlovski nmihajlovski commented Sep 2, 2016

Some users complained that the "rapidoid" tests are implemented using low-level techniques, despite the expected one-liners presented in the docs.

I renamed the apparently misleading "rapidoid" tests to "rapidoid-http-fast", to avoid the confusion and set the right expectations.

rapidoid-http-fast is the low-level HTTP and generic network protocol framework:
http://www.rapidoid.org/http-fast.html

I really believe that now the "rapidoid-http-fast" benchmark should be marked as realistic, because:

  • rapidoid-http-fast is used using low-level bytes manipulation, exactly as officially advertised on the web site,
  • it has production quality (rounds 11 and 12 demonstrated its high performance and 0 errors),
  • low-level doesn't imply "striped" (e.g. Netty).

But, to be really fair, I already opened a PR using the high-level Rapidoid API:
#2256

I hope we will be able to see both the high-level and low-level approaches and compare the results very soon...

@knewmanTE
Copy link
Copy Markdown
Contributor

@nmihajlovski, given that this merge request doesn't address the concerns that were initially raised about Rapidoid's implementation, I don't think just changing the name of the test is enough to merit reclassify it from Stripped to Realistic, as it is still manually constructing the byte arrays for all of the response headers.

@nmihajlovski
Copy link
Copy Markdown
Contributor Author

I took care of that. The old code is replaced by a new and simple code.
(Basically a made copy of the rapidoid-http-fast implementation of PR #2256.

@nmihajlovski
Copy link
Copy Markdown
Contributor Author

@knewmanTE Is it too late to include this in the upcoming "preview 2" round?

@knewmanTE
Copy link
Copy Markdown
Contributor

@nmihajlovski we ran preview 2 over the weekend, so this pull request was not in the code base for our suite run.

@nmihajlovski
Copy link
Copy Markdown
Contributor Author

@knewmanTE Is there any reason why this PR hasn't been marked with a "Round-13" label?

@knewmanTE
Copy link
Copy Markdown
Contributor

Hey @nmihajlovski, sorry for the delay. We've been busy with some internal tasks preparing for preview 2 and haven't had the time to do a full pass-through of the additional changes you've made. We'll be taking a look today though!

@knewmanTE knewmanTE added this to the Round 13 milestone Sep 16, 2016
"versus": "rapidoid"
"tests": [
{
"http-fast": {
Copy link
Copy Markdown
Contributor

@knewmanTE knewmanTE Sep 16, 2016

Choose a reason for hiding this comment

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

could you change this test name back to 'default'? The way the suite is built, each benchmark_config.json must contain at least a 'default' test.

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.

Thanks @knewmanTE for the quick reply. I changed the test name to 'default'.

@knewmanTE knewmanTE merged commit a65b3ed into TechEmpower:master Sep 16, 2016
@nmihajlovski
Copy link
Copy Markdown
Contributor Author

Thanks!

NateBrady23 pushed a commit to NateBrady23/FrameworkBenchmarks that referenced this pull request Sep 23, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants