facebook / facebook-sdk-for-unity

The facebook sdk for unity.
https://developers.facebook.com/docs/unity
Other
489 stars 256 forks source link

FacebookSDK generates excessive permanent GC memory #697

Open Razenpok opened 1 year ago

Razenpok commented 1 year ago

Checklist

Environment

Describe your dev environment here, giving as many details as possible. If you have them, make sure to include:

Code Samples & Details

As per Unity docs

Mono and IL2CPP internally cache all C# reflection (System.Reflection) objects and by design, Unity doesn’t garbage collect them. The result of this behavior is that the garbage collector continuously scans the cached C# reflection objects during the lifetime of your application, which causes unnecessary and potentially significant garbage collector overhead.

To minimize the garbage collector overhead, avoid methods such as Assembly.GetTypes and Type.GetMethods() in your application, which create a lot of C# reflection objects at runtime. Instead, you should scan assemblies in the Editor for the required data and serialize and/or codegen it for use at runtime.

FacebookSDK goes through assemblies one by one during the search for IAPButton class from UnityEngine.Purchasing:

https://github.com/facebook/facebook-sdk-for-unity/blob/b8db43b2b6006e2cf698cf42cba8205d94c71472/Facebook.Unity/Utils/CodelessIAPAutoLog.cs#L103

image

Again, the generated GC memory from Assembly.GetTypes() is permanent, as it won't ever be collected.

This issue could be fixed with the following steps:

  1. Add a method for a developer to disable the automatic invocation of addListenerToIAPButtons completely, as not all projects use codeless IAP buttons
  2. Optimize reflection code:
private static System.Type FindTypeInAssemblies(string assemblyName, string typeName, string nameSpace)
{
    var assemblies = System.AppDomain.CurrentDomain.GetAssemblies();
    for (int i = 0; i < assemblies.Length; i++)
    {
        var assembly = assemblies[i];
        if (assembly.GetName().Name != assemblyName)
        {
            continue;
        }

        var type = assembly.GetType(nameSpace + "." + typeName);
        if (type != null && typeName == type.Name && nameSpace == type.Namespace)
        {
            return type;
        }
    }

    return null;
}
MarkZaytsev commented 4 months ago

I can confirm this. It became a problem in our project. It causes 1 MB allocation by doing 16500 method calls on every scene load. And we don't even use CodelessIAP.

2024-05-28_19-26

kocburak commented 4 months ago

If you are not using CodelessIAP, Just remove it. Why bother optimize? If something goes wrong just return null. What could possible go worng :D