Avoid env-server trace recopy - #1809
Merged
Merged
Conversation
Contributor
ApprovabilityVerdict: Approved Performance optimization using Pydantic's You can customize Macroscope's approvability policy. Learn more. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Overview
Keep completed V1 traces concrete inside env-server response wrappers and defer
WireTaskconversion to the existing client boundary. This removes a redundant full-trace dump and validation pass before the unchanged wire serializer.Mechanism
episode.run()already returns typed, trustedTraceobjects. The rollout path previously dumped each concrete trace to a dictionary, then response validation rebuilt it asTrace[WireTask]; the response field serializer subsequently dumped that rebuilt trace again for msgpack. Group responses repeated the same work for every trace.The response wrappers now use
model_constructaround those trusted concrete traces. Their existing field serializers still produce the wire payload once, and the client continues validating decoded responses intoTrace[WireTask]. Request handling, response schemas, msgpack encoding, ZMQ transport, task extras, excluded state, and client-side validation remain unchanged.Performance
A PEP 723 response-path benchmark used one trace with one node and 1,825,000 token IDs, masks, and log probabilities (5,475,000 list entries total), producing a 27,244,201-byte msgpack response. Values below are medians of three fresh processes after warm-up on macOS.
The synchronous time reduction directly shortens the env-server event-loop stall for large responses. Group savings scale with the number and size of completed traces.
Note
Low Risk
Server-only response construction optimization on trusted rollout output; wire encoding and client validation paths are unchanged.
Overview
Rollout and group handlers no longer turn completed traces into dicts before building
RunRolloutResponse/RunGroupResponse. They wrap the concreteepisode.run()traces withmodel_constructinstead of going throughmodel_dump()plus full Pydantic validation on the way in.That removes an extra full-trace copy and rebuild on the server while the existing
trace/tracesfield serializers still run once when_handlecallsmodel_dumpfor msgpack.WireTaskre-typing stays on the client viamodel_validateon the decoded payload.Group responses get the same treatment per trace, so savings grow with group size and trace payload size.
Reviewed by Cursor Bugbot for commit dd1ea56. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Avoid redundant trace serialization in
EnvServerrollout and group responsesIn
server.py,_run_rolloutand_run_groupnow usemodel_constructto build responses directly from existingTraceobjects instead of callingmodel_dump()on each trace and re-validating. This skips an unnecessary serialize-then-validate round-trip per trace.Macroscope summarized dd1ea56.