PUN reducing per-frame memory allocations - advice needed

Options
luxel
luxel
Dear Photon Support,

As we know, collection objects like dictionaries and hash tables are very widely used in PUN. When the game is running at a high network update rate, the memory allocations become big (>1 KB per frame), which results in frequent GC activity. So we’re really looking forward for some improvements to reduce the memory consumption. Is there any plan for this?

I have been studying PUN code a lot, and since in games the networking loop is single threaded (because always invoked from update) (not counting the fundamental dispatching or receiving threads), there are a few places where object instantiations could be avoided and reuse existing objects.

E.g. in NetworkingPeer.RunViewUpdate:

For dataPerGroupReliable & dataPerGroupUnreliable, there is no need to create these two dictionaries overtime, since these two are only used in the RunViewUpdate method. I’m going to convert them into class wide private variables.
Dictionary<int, Hashtable> dataPerGroupReliable = new Dictionary<int, Hashtable>();
    Dictionary<int, Hashtable> dataPerGroupUnreliable = new Dictionary<int, Hashtable>();

    // this is called by Update() and in Unity that means it's single threaded.
    public void RunViewUpdate() {
     ...
// Instead of creating them everytime, we really should keep their reference and reuse.
//        Dictionary<int, Hashtable> dataPerGroupReliable = new Dictionary<int, Hashtable>();
//        Dictionary<int, Hashtable> dataPerGroupUnreliable = new Dictionary<int, Hashtable>();

E.g. LoadBalancingPeer.OpRaiseEvent (once OpCustom method finishes, the opParameters is already serialized and enqueued into the dispatching queue, so we’re safe to reuse it for next call)
private Dictionary<byte, object> opRaiseEventParameters = new Dictionary<byte, object>();

public virtual bool OpRaiseEvent(byte eventCode, object customEventContent, bool sendReliable, RaiseEventOptions raiseEventOptions)
        {
            Dictionary<byte, object> opParameters = opRaiseEventParameters;    // re-use the dictionary, assuming this method is always called single threaded.

I believe there are many more places where we can avoid memory allocations, even introducing some kind of object pool. Please let us know your advices, thank you!! (meanwhile, we're also looking at how to reduce our update rates)

Comments

  • luxel
    Options
    Some more diggings with Unity Profiler:

    Can we reuse the PhotonStream object, Hashtable and Dictionary when doing OnSerializeWrite?
  • luxel
    Options
    Can anyone help with this? Please at least help me review the change I made. (I fully understand this code change to PUN is not official and we take our own risk) ;)

    The biggest assumption I made was - in PhotonPeer.OpCustom method, when "this.peerBase.EnququeOperation" is done, the OperationRequest is fully serialized into the dispatching buffer and the dictionary/hashtable used in OperationRequest is no longer needed. Please let me know whether we can make this assumption.
  • Tobias
    Options
    Sorry for the late reply. I was sick and Vadim didn't have the time to dig into this so far.

    Your assumption is correct: Within OpCustom, all parameters are serialized and you are free to re-use any parameter you passed to it.
    We made some effort in PUN 1.50 to improve memory usage. I concentrated on the lower levels but your changes make a lot of sense, so I will take a look and might add them in upcoming releases.

    Usually, the network code should only run 10 times a second or maybe 20 times.
    You won't be able to push more through the pipes (mobile devices at least can suffer from package drop and packages arrive in chunks, so the benefit of higher send rate is ruined on the receiving end). Of course, the receive code will run more frequently to avoid local lag.

    Still, lower memory consumption is always good and your feedback is appreciated.
  • luxel
    Options
    Thank you Tobias! Your explainations helped a lot. And we have been testing our game with the changes for a while and so far no problem is found.

    And looking forward to future releases of PUN ;)
  • Tobias
    Options
    luxel:
    When you re-use the Dictionary, you need to clear it. I am not entirely sure if Dictionary.Clear() is causing less, as-much or more work than "new Dictionary<>()". If you profiled this, do you have proper numbers?
    I will put both improvements into the next PUN update most likely.
  • luxel
    Options
    I didn't profile the CPU time of Dictionary.Clear & new Dictionary, but Dictionary.Clear will cause zero allocation.