-
Notifications
You must be signed in to change notification settings - Fork 6
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
Use prototype based Recorder for performance boost. #12
Use prototype based Recorder for performance boost. #12
Conversation
value: value, | ||
events: this.captured | ||
}, | ||
source: { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why can't this just be source: args
? I did it this way to mimic what you had before, but I am not entirely sure why it's necessary to create a safe copy of args
here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah it's redundant. source: args
would be better. Renaming args
would be more better :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK. Working on this now. It takes a while to update all the "expected" fixtures
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done!
@twada - The An alternate solution (should you ever want to add code coverage to this project), would be to move the helper to it's own file and just use |
@jamestalmage Nice optimization!
|
|
@twada - should be good to go |
@jamestalmage Looks good to me. I'm going to merge and cut a new release. |
Use prototype based Recorder for performance boost.
@jamestalmage babel-plugin-espower 2.1.1 is out. Thanks! |
The current method of creating a PowerAssertRecorder prevents some optimizations by the V8 compiler.
Specifically, it creates many extra closure contexts, and prevents the use of hidden classes.
My test suite was seeing roughly a 5% boost.