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

Optimize performance of call-stack getting #1300

Merged
merged 1 commit into from
Feb 26, 2017
Merged

Optimize performance of call-stack getting #1300

merged 1 commit into from
Feb 26, 2017

Conversation

Gvozd
Copy link
Contributor

@Gvozd Gvozd commented Feb 26, 2017

call stack not actually needed at most cases, but got always
get(actually only first get) call stack from error is so slow operation, and may be moved on-demand getter

Purpose (TL;DR) - mandatory

Optimize work with callstack of stubbing
Call of stubbed functions was 5-15 times faster

Background (Problem in detail) - optional

When stubbed function call, that remember hist call stack for use in call.toString()
But get stack-property from Error-object is so slow in first getting
Also this very slow with babel, because used source-map convert of stack
I think, that not actually need get stack, when call-object constructed, but may getted when this really need(at toString() method)

Solution - optional

I not resolve stack at call-object constructring, but I remeber only generated Error-object
Only when stack actually needed, I get it from Error-object
Important: only first getting of stack from Error-object is slow. Repeated get stack is cached(I check this at Chrome)

How to verify - mandatory

  1. Check out this branch (see github instructions below)
  2. npm install
  3. Put in root of repo this file https://gist.github.com/Gvozd/291e4f0adf247ff70ed72150059e4977
  4. Run this file with node or babel-node command
  5. Performance of running this example is better, than withount my fix
    x5 times at Node.js
    x15 times at babel-node

@coveralls
Copy link

Coverage Status

Coverage remained the same at 95.421% when pulling a865362 on Gvozd:optimize_performance into 46e41c8 on sinonjs:master.

call stack not actually needed at most cases, but got always
get(actually only first get) call stack from error is so slow operation, and may be moved on-demand getter

Please see this simple example and his performance:
https://gist.github.com/Gvozd/291e4f0adf247ff70ed72150059e4977
with node:
  before commit: 810-840ms
  now: 170-180ms
with babel-node(slowe because source-map converting stack):
  before commit: 2900-3100ms
  now: 190-230ms
@coveralls
Copy link

coveralls commented Feb 26, 2017

Coverage Status

Coverage remained the same at 95.421% when pulling 6b27ce4 on Gvozd:optimize_performance into 46e41c8 on sinonjs:master.

Copy link
Contributor

@fatso83 fatso83 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems good to me. Any objections?

@cjohansen cjohansen merged commit aac0b2e into sinonjs:master Feb 26, 2017
@cjohansen
Copy link
Contributor

👍 Nice work!

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.

4 participants