Skip to content

[FR]: At least unnecessary GC allocations should be eliminated for analysis. #1226

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

Open
KarahanOnarlar opened this issue Mar 31, 2025 · 1 comment

Comments

@KarahanOnarlar
Copy link

Description

  1. For analysis activities with multiple parameters, especially if they occur frequently in the game, gc will be generated.

  2. Currently, parameters are not suitable for reuse, so they are constantly recreated unnecessarily and cause boxing. However, after being passed to the LogEvent method, they are passed to StringList and VariantList and the job is done. They can be reused if type safety is ensured. At the same time, the Parameter.Value is subject to boxing and unboxing.

  3. My suggestion is to solve both the boxing problem and create reusable parameter types with a small change in the Parameter class. This change will also not break the old usage and will provide backward compatibility.

API Proposal

1. Edit Parameter.cs

Edit the parameter class as follows. This maintains backward compatibility and also allows us to create versions that avoid boxing that are reusable.

public class Parameter
{
    // Make public instead of internal
    public string Name { get; set; }
    // Make private instead of internal.
    object Value { get; set; }
    // Add to be able to create types that help us avoid boxing.
    internal virtual Variant ConvertToVariant() { return Variant.FromObject(Value); }

    protected Parameter() { }
    public Parameter(string parameterName, long parameterValue) : this(parameterName, (object)parameterValue) { }
    public Parameter(string parameterName, double parameterValue) : this(parameterName, (object)parameterValue) { }
    public Parameter(string parameterName, string parameterValue) : this(parameterName, (object)parameterValue) { }
    public Parameter(string parameterName, IDictionary<string, object> parameterValue) : this(parameterName, (object)parameterValue) { }
    public Parameter(string parameterName, IEnumerable<IDictionary<string, object>> parameterValue) : this(parameterName, (object)parameterValue) { }
    private Parameter(string name, object value) { Name = name; Value = value; }
}

2. Edit FirebaseAnalytics.cs

public static class FirebaseAnalytics
{
    // Add a list option and use the Parameter.ConvertToVariant method instead of Variant.FromObject when passing it to VariantList.
    // Use for instead of foreach.
    public static void LogEvent(string name, List<Parameter> parameters)
    {
        StringList stringList = new StringList();
        VariantList variantList = new VariantList();
        for (int i = 0; i < parameters.Count; i++)
        {
            Parameter parameter = parameters[i];
            stringList.Add(parameter.Name);
            //Use ConvertToVariant instead of Variant.FromObject
            variantList.Add(parameter.ConvertToVariant());
        }
        FirebaseAnalyticsInternal.LogEvent(name, stringList, variantList);
    }
}

3. Create types that can be reused and avoid boxing.

public abstract class Parameter<T> : Parameter
{
    public T Value { get; set; }
    internal override Variant ConvertToVariant() { return Variant.FromObject(Value); }
    public Parameter<T> SetValue(T value) { Value = value; return this; }
    public Parameter<T> Set(string name, T value) { Name = name; Value = value; return this; }
    public Parameter() { }
    public Parameter(string name) {  Name = name; }
    public Parameter(string name, T value) { Name = name; Value = value; }
}

public class LongParameter : Parameter<long>
{
    public LongParameter(string name) : base(name) { }
    public LongParameter(string name, long value) : base(name, value) { }
    internal override Variant ConvertToVariant() { return Variant.FromInt64(Value); }
}

public class DoubleParameter : Parameter<double>
{
    public DoubleParameter(string name) : base(name) { }
    public DoubleParameter(string name, double value) : base(name, value) { }
    internal override Variant ConvertToVariant() { return Variant.FromDouble(Value); }
}

public class StringParameter : Parameter<string>
{
    public StringParameter(string name) : base(name) { }
    public StringParameter(string name, string value) : base(name, value) { }
    internal override Variant ConvertToVariant()
    {
        if (Value != null)
        {
            return Variant.FromString(Value);
        }
        return Variant.Null();
    }
}

4 Optional: Support Dictionary Types

new Parameter(string name, IDictionary<string, object)
new Parameter(string name, IEnumerable<IDictionary<string, object>)
Optimized codes for these two types are ready, but I am not writing them here for now to avoid confusion. My priority is for you to add the types above. If you are interested, I can also put the optimized codes of both types here. Just let me know.

5. Usage

Now we can cache parameters as we wish and avoid constantly allocating memory.

readonly StringParameter platform = new StringParameter("ad_platform", "ironSource");
readonly StringParameter source = new StringParameter("ad_source");
readonly StringParameter unitName = new StringParameter("ad_unit_name");
readonly StringParameter adFormat = new StringParameter("ad_format");
readonly StringParameter currency = new StringParameter("currency", "USD");
readonly DoubleParameter value = new DoubleParameter("value");
readonly List<Parameter> adParameters = new List<Parameter>();

private void ImpressionSuccessEvent(IronSourceImpressionData impressionData)
{
    if (impressionData != null)
    {
        adParameters.Clear();
        adParameters.Add(platform);
        adParameters.Add(source.SetValue(impressionData.adNetwork));
        adParameters.Add(unitName.SetValue(impressionData.instanceName));
        adParameters.Add(adFormat.SetValue(impressionData.adFormat));
        adParameters.Add(currency);
        adParameters.Add(value.SetValue(impressionData.revenue ?? 0d));
        
        Firebase.Analytics.FirebaseAnalytics.LogEvent("ad_impression", adParameters);
    }
}

Firebase Product(s)

Analytics

Targeted Platform(s)

No response

@google-oss-bot
Copy link

I couldn't figure out how to label this issue, so I've labeled it for a human to triage. Hang tight.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants