adamncasey
commented
2 years ago Results for connection throughput testing - as before, I'm really just trying to make sure we don't regress performance here, and comparing against the not-shown fix using a fixed buffer size. The memory usage is fairly adhoc - I just watched top while the tests ran.
Values in connections completed per second over the test run:
| Test # | TLS | TLS | TLS |
|---|---|---|---|
| Code | Current | Fixed Buffer 32K | Tiny Buffer +ask |
| 1 | 928 | 876 | 963 |
| 2 | 896 | 923 | 963 |
| 3 | 909 | 860 | 887 |
| memory | 1.3% | 2.0% | 1.1% |
The machine used for testing had 16GB of ram - so assuming top isn't doing something crazy 1.1% is about 180MiB and 2% is about 320MiB.
$ target/release/amqpprox_perf_tester --address 'amqp://127.0.0.1:16001' --listen-address '0.0.0.0:5671' --message-size 1 --num-messages 1 --listen-cert ../../build/Linux/cert.pem --listen-key ../../build/Linux/key.pem --clients 10000 --max-threads 150Each test run was only about 10 seconds. I hit ephemeral port issues on this host running longer tests - probably can work around this if we want to, but these numbers seemed like a reasonable comparison to me.
asio::sslhas a bug where calling async_read_some withnull_buffersisn't correctly handled, and more or less immediately invokes the provided handler with no data. This causesamqpproxto busy-loop whenever a TLS socket has been accepted or opened.There were two options for how to address this in our code:
1) Always read with a fixed size buffer, such as 32kb. This would simplify the code slightly, at the expense of needing multiple reads to handle larger than 32kb frames in non-TLS mode, even when the full frame is available to
amqpproxin one go. 2) Ask asio::ssl to read with a very small buffer, then ask the openssl library how many bytes are available. This technique aligns with howamqpprox's read loop works today. That is what is implemented here.In theory something similar could be upstreamed into
asio::ssl. It's a little tricky though and this exact code couldn't handle the genericMutableBufferSequenceinterface - we can take some shortcuts in our code.I've done some benchmarking to check this change isn't going to regress performance noticeably. Data throughput tests indicate that this fix improves performance for TLS connections over the existing code.
Performance Tests
Testing by setting up amqpprox as per the performance tests README.md, and running the perf tester like:
Results in MiB/s over a ~20second run
This shows there isn't really any chance to non-TLS performance, and there is a slight increase in throughput with this code change (or the fixed-buffer size change).
Still TODOasync_read_somedefinition since it should be unused[x] Run connection throughput tests. Measure memory usage for thousands of outstanding connections.
Ref: https://github.com/chriskohlhoff/asio/issues/1015