-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
remove unnecessary allocs in SideChannelMananger #4886
Conversation
{ | ||
// Early out so that we don't create the MemoryStream or BinaryWriter. | ||
// This is the most common case. | ||
return Array.Empty<byte>(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't try this with null instead of Empty(), but I'm 99% sure the protobuf code will choke on null.
@@ -30,6 +30,9 @@ removed when training with a player. The Editor still requires it to be clamped | |||
#### com.unity.ml-agents (C#) | |||
- Fix a compile warning about using an obsolete enum in `GrpcExtensions.cs`. (#4812) | |||
- CameraSensor now logs an error if the GraphicsDevice is null. (#4880) | |||
- Removed unnecessary memory allocations in `ActuatorManager.UpdateActionArray()` (#4877) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@surfnerd I added our other PRs here. If this gets repetitive, we can group these into a nested list later.
Proposed change(s)
How it started
How it's going
This early-outs from GetSideChannelMessage if there are no messages to process. This should be the case the vast majority of the time, especially during inference (unless the user is actively doing something like calling
StatsRecorder.Add()
).Thanks @szantner for reporting!
Useful links (Github issues, JIRA tickets, ML-Agents forum threads etc.)
#4883
Types of change(s)
Checklist
Other comments