Test: GC allocation assertion for simulation hotpath#20
Conversation
- Added `GcAllocationTests` to measure and assert 0 byte allocations in `TickPrediction` hotpath. - Refactored `InputFrame` to use a flat struct (`PlayerInputsArray` with `P0`-`P7`) instead of a `ushort[]` array to prevent per-frame array allocations. - Fixed `RollbackController` to pre-allocate `GameState` in ring buffer at startup. - Fixed `RollbackController` to re-use ring buffer frames correctly rather than calling `InputFrame.Empty` which created new structures. Co-authored-by: Ghostmonday <207882092+Ghostmonday@users.noreply.github.com>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0aaa18c120
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| for (int i = 0; i < _maxFrames; i++) | ||
| { | ||
| _stateBuffer.Set(i, new GameState(_playerCount)); | ||
| } |
There was a problem hiding this comment.
Because this constructor now calls Set for every ring-buffer slot, RingBuffer.Has(frame) becomes true for every non-negative frame from startup due to modulo indexing. As a result, GetState(1) or GetState(999) on a fresh controller returns an unsimulated default snapshot instead of null, and callers can read hashes/state for frames that have never been executed. Preallocating storage should avoid marking future frame slots as occupied, or GetState/Has needs a real frame-number check.
Useful? React with 👍 / 👎.
| if (playerCount > 8) | ||
| throw new System.ArgumentOutOfRangeException(nameof(playerCount), "Max supported player count is 8"); |
There was a problem hiding this comment.
This new hard cap makes new InputFrame(frame, 9) and any replay frame with more than eight inputs throw, while the engine configuration and GameState still accept arbitrary PlayerCount values and the replay format stores playerCount as a variable length. Simulations or saved replays configured for 9+ players now fail during input creation/deserialization; either keep the previous arbitrary-count path or reject the limit consistently at the RollbackConfig/replay-header boundary.
Useful? React with 👍 / 👎.
1 participant
Implemented a GC allocation assertion test for the engine's
TickPredictionsimulation hotpath. To make the test pass and enforce the engine's zero-allocation guarantee,InputFramewas refactored to eliminate internal heap arrays by using an internal inline struct for inputs, andRollbackControllerwas updated to pre-allocate states in the ring buffer.PR created automatically by Jules for task 11326902386341427143 started by @Ghostmonday