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

Remove unnecessary fields from the MeshBVHNode to save memory #18

Closed
gkjohnson opened this issue Sep 10, 2018 · 4 comments
Closed

Remove unnecessary fields from the MeshBVHNode to save memory #18

gkjohnson opened this issue Sep 10, 2018 · 4 comments

Comments

@gkjohnson
Copy link
Owner

Tons of nodes get created so any reduction we can make per object (even empty dictionary keys) may be worthwhile.

  • _mesh does not necessarily need to be a pointer on every node.
  • children can be removed if the node is a leaf node (or only be added if it's an internal one)
  • tris can be removed if the node is an internal node.
@mqp
Copy link
Contributor

mqp commented Sep 11, 2018

It's possible that removing fields conditionally (and thereby making all the nodes not have homogeneous shapes) could hurt performance due to JIT inline caching, so it's worth benchmarking after this to make sure there's not a regression.

@gkjohnson
Copy link
Owner Author

That's a good point. If it does wind up being slower then having two different classes may be a better option? One with children and one with tris?

I'll also have to see how much removing a key really saves here. I'm not so familiar with how to estimate memory use when it comes to javascript object keys.

@mqp
Copy link
Contributor

mqp commented Sep 12, 2018

No idea if the objects having the same prototype will make a difference or not.

Removing children is particularly appealing since for leaf nodes it's an empty array, so we're paying memory twice -- once for the pointer and once for the empty array. tris is just null for non-leaves so we only pay for the pointer.

@gkjohnson
Copy link
Owner Author

Most unneeded fields removed in #44

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