Skip to content

[Performance] Reduce Memory footprint for tracks#1049

Merged
subdavis merged 2 commits into
mainfrom
busfix
Nov 17, 2021
Merged

[Performance] Reduce Memory footprint for tracks#1049
subdavis merged 2 commits into
mainfrom
busfix

Conversation

@subdavis
Copy link
Copy Markdown
Contributor

@subdavis subdavis commented Nov 15, 2021

Removes a Vue instance from each track instantiation, replace with a simple callback function.

Profile results

With 100K tracks and 20 types on 20 image frames

  • Before: 535-538MB (consistent) memory usage at idle, tops out at 700+ during load
  • After: 305-311MB (consistent) memory usage at idle, tops out at 450 during load.

App uses 80MB of memory with no tracks at idle, so this results in about a 50% reduction in dynamic memory usage.

Comment thread client/src/track.ts

bus: Vue;
/** A callback to notify about changes to the track. */
notifier?: TrackNotifier;
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking at this now is it necessary for each Track to have it's own notifier. Could it be a static property and setNotifier be a static method that is assigned once for the entire class? setNotifier is always setting the same function to each track which depends on the function passing in it's own values into it? This would remove the need to unset the notifier from removed tracks because they no longer exist? I guess the worry would be that some track after calling removeTrack still calls a notify even though it isn't in the trackMap? Let me know if there are things I haven't thought of and counter examples.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After thinking about it, I don't think we should make it a static class method. Looking ahead to stereo or multi-camera views, we may need to be able to differentiate between the event sourcees.

The property is just a pointer, so it costs almost nothing in memory for each track to hold a reference (8 bytes for a pointer).

So for 100K tracks, we're talking 800KB.

Copy link
Copy Markdown
Collaborator

@BryonLewis BryonLewis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The justification for future usage is enough to keep it as is. I vaguely thought that there may be a need for multi-stereo but had not fully thought it out.

@subdavis subdavis merged commit dce5857 into main Nov 17, 2021
@subdavis subdavis deleted the busfix branch November 17, 2021 19:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants