Search Unity

Bug - HashSet.TrimExcess causes an increase in memory, opposite of what it should do.

Discussion in 'Scripting' started by HiddenMonk, Nov 17, 2017.

  1. HiddenMonk

    HiddenMonk

    Joined:
    Dec 19, 2014
    Posts:
    987
    Bug report number 969832
    The bug was tested and is present in both Unity 2017.1.0p4 and the current version 2017.2.0f3
    Edit - Tested in Unity 2017.2.0f3 with the experimental 4.6 c# there seems to be no issues.

    It seems that every time you call TrimExcess on a hashset, while its suppose to decrease the backend collection size to better fit your hashset current size, there seems to be a bug where it actually just increases the backend collection size.
    Here are the logs
    HashsetBugLogs.png

    Here is the test code.
    Just place it on a gameobject, press play, and keep repeatedly toggling enable to see the Debug.Logs show it increasing.
    Code (CSharp):
    1.  
    2. using System;
    3. using UnityEngine;
    4. using System.Collections.Generic;
    5. using System.Reflection;
    6.  
    7. public class TestHashSetTrimExcessBug : MonoBehaviour
    8. {
    9.     HashSet<int> hashset = new HashSet<int>() {1};
    10.  
    11.     void OnEnable()
    12.     {
    13.         hashset.TrimExcess();
    14.  
    15.         BindingFlags flags = BindingFlags.NonPublic | BindingFlags.Instance | BindingFlags.GetField | BindingFlags.GetProperty;
    16.         int hashThreshold = (int)hashset.GetType().GetField("threshold", flags).GetValue(hashset);
    17.         int hashSlotCount = ((int[])hashset.GetType().GetField("slots", flags).GetValue(hashset)).Length;
    18.  
    19.         Debug.Log("Hashset - Trimming Excess. Count = " + hashset.Count + ", Capacity = " + hashThreshold + ", Slot count = " + hashSlotCount);
    20.     }
    21. }

    I was met with this bug thanks to another bug where clicking on materials or gameobjects with materials causes SceneManager.onSceneUnloaded event to be called in the editor (That bug seems to be fixed in Unity 2017.2.0f3).
    I had a method assigned to onSceneUnloaded that would call TrimExcess on 10 or so hashsets. After playing in the editor and clicking on a few gameobjects, unity would increasingly slow down each gameobject I click and eventually just freeze my computer, needing me to do a hard shutdown. After a couple of times of not knowing why this was happening, I actually got a warning from windows that my disks had corrupted files and it needed to scan and do a cleanup or something, so that was a little scary.
    I eventually narrowed it down to the HashSet.TrimExcess causing the issue, but didnt know why. I searched online and didnt find anything for unity, but when searching for HashSet.TrimExcess bug causing a crash, I found a Xamarin bug report saying how TrimExcess was increasing memory, and sure enough it seems to be true in unity as well. I also saw a tweet by a Rust Developer or something in 2015 questioning why they are crashing from HashSet.TrimExcess/Clear (I dont see anything wrong with clear, so its probably just the TrimExcess).
     
    Last edited: Nov 18, 2017
  2. JoeStrout

    JoeStrout

    Joined:
    Jan 14, 2011
    Posts:
    9,859
    I think you're barking up the wrong tree. HashSet is not part of the Unity framework, and you already found somebody in the Xamarin community complaining about the same thing — clearly this is either a bug in the Mono framework, or a misunderstanding about what TrimExcess is supposed to do.

    In any case, while I guess there's no harm in reporting it, I wouldn't really expect Unity to fix this. I suggest you just quit calling TrimExcess.
     
  3. nat42

    nat42

    Joined:
    Jun 10, 2017
    Posts:
    353
    Looks to fit some version of Mono eg. https://searchcode.com/codesearch/view/35723789/ but not the current version which looks much better https://github.com/mono/mono/blob/0...em.Core/System/Collections/Generic/HashSet.cs ?

    Can't really report a bug to the Mono devs if they fixed it years ago, but it hasn't made it to Unity's internal version (assuming that is the case here?)
     
  4. JoeStrout

    JoeStrout

    Joined:
    Jan 14, 2011
    Posts:
    9,859
    Quite likely. I believe Unity is still using roughly Mono 2.6 or so. I believe there are some fairly deep technical issues that make it difficult for them to update to a newer runtime.
     
  5. KelsoMRK

    KelsoMRK

    Joined:
    Jul 18, 2010
    Posts:
    5,539
    Does this still exist if you use the 4.6 runtime?
     
  6. HiddenMonk

    HiddenMonk

    Joined:
    Dec 19, 2014
    Posts:
    987
    Perhaps, but I thought unity makes their own edits to mono framework when needed, such as the classic null check override for unity objects.
    Either way, its at least a warning for others to not use it.

    Tested in Unity 2017.2.0f3 with the experimental 4.6 c# there seems to be no issues.
    I guess its a good time to consider using the new c# (didnt realize it was out yet).

    Heres the test code.
    Had to make small changes to the test code for the new c# version.
    Code (CSharp):
    1.  
    2. using System;
    3. using UnityEngine;
    4. using System.Collections.Generic;
    5. using System.Reflection;
    6.  
    7. public class TestHashSetTrimExcessBug : MonoBehaviour
    8. {
    9.     HashSet<int> hashset = new HashSet<int>() {1};
    10.  
    11.     void OnEnable()
    12.     {
    13.         hashset.TrimExcess();
    14.  
    15.         BindingFlags flags = BindingFlags.NonPublic | BindingFlags.Instance | BindingFlags.GetField | BindingFlags.GetProperty;
    16.  
    17.         //c# 4.6
    18.         int hashBucketCount = ((int[])hashset.GetType().GetField("m_buckets", flags).GetValue(hashset)).Length;
    19.         int hashSlotCount = ((Array)hashset.GetType().GetField("m_slots", flags).GetValue(hashset)).Length;
    20.         Debug.Log("Hashset - Trimming Excess. Count = " + hashset.Count + ", Bucket Count = " + hashBucketCount + ", Slot count = " + hashSlotCount);
    21.     }
    22. }