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

Fixed inefficient way of resetting an object #321

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

issy123
Copy link

@issy123 issy123 commented Apr 11, 2018

Looping through all object keys to delete them is very inefficient, rather remove the reference to the object and create a new object. The garbage collector is efficient enough to handle this.

Looping through all object keys to delete them is very inefficient, rather remove the reference to the object and create a new object. The garbage collector is efficient enough to handle this.
@englercj
Copy link
Contributor

englercj commented Apr 13, 2018

Just for some historical context, the reason I did it this way was because there actually were GC pause issues with allocating a new object. reset() is a hot function that is run multiple times each frame, which if it allocates generated a lot of garbage very quickly.

The compromise was to be slightly slower to run, but consistent. Rather than sometimes being faster, but spiking on GC.

@issy123
Copy link
Author

issy123 commented Apr 17, 2018

Well it is significant faster I my experience with node v6+.

What would you suggest?

@englercj
Copy link
Contributor

I always suggest measuring it, a benchmark suite that proves one thing or another is faster is always great.

@Nek-
Copy link

Nek- commented Apr 23, 2018

We need a benchmark for many browsers and different versions of nodejs for this one :/ .

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