Skip to content

Poor Runtime Performance #12

Open
Open
@RamiAhmed

Description

@RamiAhmed

Hi,

I've noticed that the way this SDK wants to track events causes a number of unfortunate memory allocations, which then need to be garbage collected and thus incurs a performance cost.

Since the TrackEvent method passes the props dictionary on to the Event, it means that we cannot use a shared dictionary for different instances of events, since the reference then becomes 'shared' and we run into issues with one event overriding the custom properties of another event.

        public static void TrackEvent(string eventName, Dictionary<string, object> props = null)
        {
            if (string.IsNullOrEmpty(_baseURL))
                return;
            
            props ??= new Dictionary<string, object>();
            var eventData = new Event()
            {
                timestamp = DateTime.UtcNow.ToString("o"),
                sessionId = EvalSessionId(),
                eventName = eventName,
                systemProps = _env,
                props = props // <--- Reference passed on as-is
            };
            
            _dispatcher.Enqueue(eventData);
        }

This means we basically have no other choice than newing up a dictionary instance for each event, which causes memory allocations that then need to be garbage collected - thus incurring a significant performance cost when there are many instances of events. One solution would be to use Unity's native collections (e.g. NativeHashMap<TKey, TValue>) and then disposing manually after flushing, however that would create a dependency on Unity.Collections which may not be desireable. Dictionary pools could be another solution, although the memory used to simply represent potentially thousands of dictionaries through a pool could also be an undesireable cost. Not sure what other solutions may exist outside of manual memory allocations to avoid the garbage collection.

Additionally, the Dispatcher.Flush() implementation creates at least 2 lists on every call, potentially more depending on the number of queued events. These allocations could easily be avoided by using cached lists or a list pool.

        public async void Flush()
        {
            if (_flushInProgress || _events.Count <= 0)
                return;

            _flushInProgress = true;
            var failedEvents = new List<Event>(); // <--- Allocation
            
            //flush all events
            do
            {
                var eventsCount = Mathf.Min(MAX_BATCH_SIZE, _events.Count);
                var eventsToSend = new List<Event>(); // <--- Allocation
                for (var i = 0; i < eventsCount; i++)
                    eventsToSend.Add(_events.Dequeue());

                try
                { 
                    var result = await SendEvents(eventsToSend);
                    if (!result) failedEvents.AddRange(eventsToSend);
                }
                catch
                {
                    failedEvents.AddRange(eventsToSend);
                }

            } while (_events.Count > 0);
            
            if (failedEvents.Count > 0) 
                Enqueue(failedEvents);

            _flushInProgress = false;
        }

Am I missing something? or is there some type of workaround for this? My plan - in the absence of better suggestions - is to make a fork of this project and change it to use NativeHashMap<TKey, TValue> as that would allow us to create a new map for each event and then dispose it, thus circumventing the allocation issues. Would love to hear any other solutions!

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions