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

Adding http4k benchmark #451

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

kristian-petras
Copy link

This MR is adding new benchmark to already existing renaissance suites.

Benchmark is creating basic application that manages products using http4k and simulates workload using multiple parallel client requests.

Merge request also introduces kotlin into the renaissance ecosystem. Other contributors now have an example on how to include kotlin based benchmarks without any hassle.

@vhotspur
Copy link
Member

Hello and thanks for your contribution (and sorry for not getting back earlier). It is great to see new language in Renaissance :-).

Overall, this looks great but I have some questions and comments that I will try to add directly to the source code.

Copy link
Member

@vhotspur vhotspur left a comment

Choose a reason for hiding this comment

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

I have tried to run the benchmark with insanely huge numbers of threads and I was unable to get the CPU utilization at 100%.

For comparison, if I run finagle-http -o client_count=1000 I get the CPU utilization higher than the amount of CPUs: I have 24 CPU beast and uptime reports values above 40.

But even with http4k -o max_threads=100 -o workload_count=20000 I never get above 15 (and I have let it run for a looong time).

Any idea what I am doing wrong?

build.sbt Outdated
name := "http4k",
commonSettingsNoScala,
kotlinVersion := "2.0.0",
kotlincJvmTarget := "21",
Copy link
Member

Choose a reason for hiding this comment

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

I have changed this locally to "1.8" so that it can run on older JVM as well. Will that break anything (I mean, the benchmark runs but perhaps the generated class file is not optimized in some way)?

Copy link
Author

Choose a reason for hiding this comment

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

I did not have any strong reasons to have it other than optimized memory management etc. If compatibility is the priority then I do not have any reason to leave it on 21. Changed.

Copy link
Member

Choose a reason for hiding this comment

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

Thanks! So perhaps can we patch this at compile time somehow with the "current" JVM version? Because default seems to be 1.6 which is probably too conservative for us.

@lbulej What do you think about which version to put here when generating the bytecode?

@kristian-petras
Copy link
Author

About the comment for CPU utilization, I am not sure what might be the issue. I have ran the benchmark with $cpu.count and I had stable >80% utilization on all cores.

Tried with Corretto 17 if it helps in any way.

@kristian-petras
Copy link
Author

Endnote: It might be good to introduce linter/formatter even for kotlin sources. I have experience only with ktlint but I can leave the selection up to the maintainers (you).

Currently, as far as I understand, only Scala sources are formatted.

@vhotspur
Copy link
Member

@kristian-petras Thanks for the updates!


About the comment for CPU utilization, I am not sure what might be the issue. I have ran the benchmark with $cpu.count and I had stable >80% utilization on all cores.

I was around 70% which for me feels like the benchmark is not benchmarking enough :-). My point is really that on finagle-http with enough threads I can overload the machine while with this one I cannot. But it is not a critical issue; I am just wondering what is so fast about it.

Endnote: It might be good to introduce linter/formatter even for kotlin sources. I have experience only with ktlint but I can leave the selection up to the maintainers (you).

Currently, as far as I understand, only Scala sources are formatted.

Yes (we tried Java ones but we were not happy with the result of the automated formatting).

I see that ktlint has sbt plugin but I wonder if we should try Spotless instead (via this plugin) to unify the checks under a single plugin that can do Java, Scala and Kotlin (and other languages too if needed).


I believe that in the current state the benchmark can be merged (assuming the CI finishes in the green) but I am not sure about next release and whether we should not postpone the inclusion into next release. Perhaps @lbulej can comment on that?

@kristian-petras
Copy link
Author

Hello. I have tested it on macbook m2 pro and i had utilisation over 100% (on battery mode).

Also I have added test configuration, which should hopefully get the pipelines running successfully. :)

@vhotspur
Copy link
Member

I have tested it on macbook m2 pro and i had utilisation over 100% (on battery mode).

I can get high utilization on my notebook (4 cores only) too but still no luck on the big machine.

I am still wondering why is that.

But I found that in http4k/.../src/.../Undertow.kt there is the following:

val server = Undertow.builder()
                .addHttpListener(port, "0.0.0.0")
                .setServerOption(ENABLE_HTTP2, enableHttp2)
                .setWorkerThreads(32 * Runtime.getRuntime().availableProcessors())
                .setHandler(handlerWithSse).build()

Which means that the clients (and their amount we control through -o max_threads) are pushing their requests against 32 * $cpu.count server threads?

I wounder if we should limit this somehow as well but I do not see an easy way to plug into this configuration (I found there is the property server.undertow.threads.io but that seems to apply only to Spring-boot framework as it had no impact on the overall utilization).

By any chance, do you happen to know if there is an easy way to change this or would we need to keep the whole server setup in our code?

As I said, this is not that critical but I would love to know if we can do something about this :-)

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.

3 participants