perf(runtime): add request-plane msgpack payload codec - #10437
Conversation
WalkthroughThis PR introduces configurable request-plane payload encoding: an environment variable selects between JSON and Msgpack codecs, the choice is embedded in request control messages and cached globally, and both egress (sending requests, receiving responses) and ingress (receiving requests, sending responses) paths use the codec to serialize and deserialize typed payloads. ChangesRequest-plane codec selection and integration
🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
|
If you run the test 5 times, how much variance are you getting? Those numbers look like they could be within measurement error. Parsing JSON is expensive (you have to read it byte-by-byte) so I support us changing it. We could do our own binary format, it's easy. The forward direction (frontend -> backend) does that. msgpack is fine too. FlatBuffers https://crates.io/crates/flatbuffers might be worth a try. |
|
@grahamking Results from 5 runs:
|
|
It's better. Let's make if the default. |
This comment has been minimized.
This comment has been minimized.
I don't think we can do that. We need Dynamo 1.3 to be backwards compatible with 1.2, so we can't switch the codec until both frontend and backend already have it. |
8747bca to
f9323e1
Compare
Can you elaborate on "both frontend and backend already have it"? This codec change covers both ingress and egress so both sides should have it. Are you worried about the mix of use between Dynamo 1.3 and 1.2? |
f9323e1 to
73903bd
Compare
|
Will merge this in for now, and make this the default after the 1.3.0 code freeze. |
|
@jthomson04 @GuanLuo do you think this pr can be cherry-picked on earlier versions of dynamo? |
|
@michaelfeil Can you clarify the ask here? Are you looking for just a branch off of a release version with that fix, or also published/republished containers/wheels? We generally don't support re-publishing existing releases, but I can put together a branch off a release version with this PR that you can build from. |
(cherry picked from commit ac51fc8) Signed-off-by: Yuewei Na <nv-yna@users.noreply.github.com>
Summary
Benchmarked Perf
Measured on the agentx trace at concurrency 512, using fast mockers:
DYN_REQUEST_PLANE_CODEC=msgpackObserved uplift from the transport switch in that setup:
Testing
Summary by CodeRabbit
New Features
Tests