Skip to content
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

Investigate performance/memory overhead of adding fields and mutator keys to Node #1096

Closed
zepumph opened this issue Oct 16, 2020 · 2 comments

Comments

@zepumph
Copy link
Member

zepumph commented Oct 16, 2020

@samreid and I have been adding constructor fields and mutator keys to Node for features implemented in #1046 and #1047. We feel like it is important to quantify performance/memory overhead to weigh certain tradeoffs in that issue.

For our purposes, we are most interested in:

  • Startup slowdown when adding mutator keys (which are iterated through on every mutate call).
  • Heap snapshot change when instantiating many Nodes with extra constructor fields defined.
@zepumph zepumph self-assigned this Oct 16, 2020
@zepumph
Copy link
Member Author

zepumph commented Oct 16, 2020

After about an hour of testing we have some results to share!

For startup time, we tested by adding a parallel loop in mutate that has a no-op that can't be optimized out. We then captured a cumulative time of how long it took to loop through all keys each time Node.mutate was called during the startup of waves intro. We found that looping with the current number of keys took ~7ms in total. When we doubled the number of Node mutator keys, it took around ~8.5-9ms. This to us seems negligable, and not important to optimize for.

Next we timed the cumulative time on startup of the actual loop in mutate to see if doubling the number of mutators showed any difference in context. We did not see reproduceable differentiation (in ms):
double number of mutators
340
266
291
293
usual number of mutators
265
266
291
313

Next we created a loop that added 1000 extra Nodes in waves intro, and added them as children to the screen view, totaling 4513 in waves intro.

The snapshot after startup without any additional Node properties defined was 58.1MB

With 80 extra Node properties initialized to null, 60.0MB

The math we then did was
1.9/80/4513 = 5 bytes per property.
1.9MB/80 fields/ 4513 Nodes = 5 byes per property.

Maybe it should be 8 bytes per reference?

In general, these results to us indicate that there is tolerance, and that adding this to Node is not a bottleneck to performance or memory. That said, we would love to have @jonathanolson review these findings and comment on the validity of the methods, results, and assumptions going forward.

As it pertains directly to #1047, this makes us confident in adding two more Node fields and two more mutator keys (to support opacity in a consistent way with pickable).

@jonathanolson
Copy link
Contributor

Those seem like definitely acceptable numbers, sounds good to me!

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

2 participants