-
Notifications
You must be signed in to change notification settings - Fork 1
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
Falcon fails under websocket-bench #1
Comments
Thanks for sharing! Yeah, the Falcon version is not really optimized; it's just a reference implementation for Samuel to build a proper thing 🙂. My goal was only to make conformance tests pass. |
Sounds good. Is there a sense of how much runway is left before this needs to be in and finalized? |
I haven't had a lot of time to look into this yet, but I will. |
Circling back on this - I notice that the results also get better with no other changes than forking: require "async/container"
class BenchmarkServer
def self.run!
container = Async::Container.new
websocket_endpoint = Async::HTTP::Endpoint.parse("http://127.0.0.1:8080/cable").with(reuse_address: true)
bound_endpoint = Sync { websocket_endpoint.bound }
container.run(count: 32) do |instance|
Async do |task|
action_cable_server = ActionCable.server
action_cable_server.instance_variable_set(:@executor, ActionCable::Async::Executor.new)
app = Falcon::Server.middleware(ActionCable::Async::App.new(server: action_cable_server))
server = Falcon::Server.new(app, bound_endpoint, protocol: websocket_endpoint.protocol, scheme: websocket_endpoint.scheme)
server.run
instance.ready!
puts "Server is ready and listening on #{bound_endpoint.inspect}"
task.children.each(&:wait)
end
end
end
end Which results in:
This doesn't seem like it should be necessary. But just more information to look at. It should be based on processor count but this is a more accurate comparison with everything else since they're all forked by default. Still shouldn't be dropping requests, but it'll be more apples to apples once it's fully working. |
I've been investigating the issues. Puma also fails if you start it with a single process. There is something odd about the benchmark. I don't know what yet, but I'll continue to investigate. |
Okay, one issue is the Using my current implementation of Falcon
The number of broadcasts seems to fluctuate on every server I tested... is that expected? Puma
Not sure if this is a fair comparison yet. |
For both of these, I was running a single process for the server. In fact, I'm not sure it should matter that much, but I suppose for fairness, we should just use whatever is the default multi-process and max out all CPU cores (e.g. maximum connections/core). |
Maybe, some clients dropped during benchmark (though there must be some error logs in this case). What looks more interesting is the difference between consecutive steps:
From ~1s to ~10ms 🤔 |
Running falcon using
If you run websocket bench, it misses almost every broadcast:
If you modify
Async::Executor
to run an async reactor in a separate thread, things get a little better:It takes longer to run (like a minute instead of the seconds everything else takes), but the numbers are better:
Not saying this is correct per se, but definitely seem to be some issues with how it's currently setup. I'm still looking at it a bit as is @ioquatix but I wanted to post it here in the meantime.
The text was updated successfully, but these errors were encountered: