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 two array allocations per deepEqual call (critical path) #3115

Merged
merged 1 commit into from
Oct 5, 2017

Conversation

ngokevin
Copy link
Member

@ngokevin ngokevin commented Oct 3, 2017

Description:

From the browser performance tab, deepEqual was a large memory allocator. After this patch, deepEqual no longer even registers on the memory tab.

utils.deepEqual is called on every single component update. Saves two arrays per component update. For things like look-controls which updates pos/rot every frame, saves a lot.

Changes proposed:

  • Don't use Object.keys in utils.deepEqual, use the same two arrays.

@dmarcos
Copy link
Member

dmarcos commented Oct 3, 2017

Failing tests

@ngokevin ngokevin force-pushed the optimizedeepequal branch 2 times, most recently from c4b0e74 to f617c88 Compare October 4, 2017 18:22
@ngokevin
Copy link
Member Author

ngokevin commented Oct 4, 2017

Fixed, I moved the object pooling utility from the other PR since the deepEqual call can get recursive, there's an uncertain amount of arrays we'd need to create.

@dmarcos dmarcos merged commit f598665 into aframevr:master Oct 5, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants