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

Improve "apply" trap performance #48

Closed
fabiospampinato opened this issue Feb 11, 2020 · 5 comments
Closed

Improve "apply" trap performance #48

fabiospampinato opened this issue Feb 11, 2020 · 5 comments
Assignees

Comments

@fabiospampinato
Copy link

I'm kind of rewriting this library to better suit my needs and I think I've found an interesting performance optimization.

Some methods of builtin objects can't mutate the object in question, like for instance calling forEach on an Array will never mutate the array, so we can avoid cloning, performing potentially expensive equality checks comparisons etc.

This is assuming the prototype of Array doesn't get messed up with, but IMHO if somebody changes it in a way that non-mutating methods become mutating it's on them.

@fabiospampinato
Copy link
Author

Actually those methods don't need to be proxied in the first place, in my benchmarks this improves performance 10x.

@fabiospampinato
Copy link
Author

Actually those methods don't need to be proxied in the first place, in my benchmarks this improves performance 10x.

Actually some of those methods need to be proxied, as otherwise for example mutating an object retrieved via forEach won't be recognized as a change.

This is still a ~4x performance improvement according to my benchmarks though.

@DarrenPaulWright
Copy link
Collaborator

Actually those methods don't need to be proxied in the first place, in my benchmarks this improves performance 10x.

Actually some of those methods need to be proxied, as otherwise for example mutating an object retrieved via forEach won't be recognized as a change.

This is still a ~4x performance improvement according to my benchmarks though.

Are you just whitelisting specific methods? Pull requests are welcome :)

@fabiospampinato
Copy link
Author

fabiospampinato commented Feb 13, 2020

Are you just whitelisting specific methods?

Essentially, yeah. Here's the list.

I'm cutting corners a bit by not even checking what kind of objects have those methods, I'll see how that goes.

Pull requests are welcome :)

Thanks but as I basically ended up rewriting the entire thing (in hindsight I probably could have just forked this and submitted a bunch of PRs, but one of the main features I wanted my library to have in the end I couldn't implement) I won't really use on-change so I don't want to kind of maintain it too.

@DarrenPaulWright
Copy link
Collaborator

Added in 2.1.0

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