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

Add caching #7

Closed
RReverser opened this issue Sep 16, 2014 · 4 comments · Fixed by #9
Closed

Add caching #7

RReverser opened this issue Sep 16, 2014 · 4 comments · Fixed by #9

Comments

@RReverser
Copy link
Contributor

In most cases, exported function returns same true value. However, every time we call it, it has to get this value somewhere from engine's depths.

So I decided to do some investigations on how could we optimize performance in V8. Here are the results:
http://jsperf.com/local-true-vs-engine-true

As you can see, we can improve performance by simply caching value of true in local variable, and performance is critical for such a frequently used function.

Thoughts? Should we use such optimization or are there even better solutions?

@mde
Copy link
Owner

mde commented Sep 16, 2014

Sounds like a pretty simple optimization, and a pretty obvious improvement. Could you put together a PR?

RReverser added a commit to RReverser/true that referenced this issue Sep 16, 2014
@RReverser RReverser mentioned this issue Sep 16, 2014
@mde mde closed this as completed in #9 Sep 16, 2014
@Manishearth
Copy link
Contributor

I'm pretty baffled by why this works. Could someone explain this further? Why is returning a constant boolean slower than returning a variable?

I'm not very clear on " it has to get this value somewhere from engine's depths."; both V8 and Spidermonkey have a separate JS value for true (JSTrue in SM)

@Manishearth
Copy link
Contributor

This is actually much slower when caching is added to the mix

http://jsperf.com/local-true-vs-engine-true/2

@mde
Copy link
Owner

mde commented Mar 24, 2016

Thank you for doing some more research and benchmarking around this. Micro-optimizations like this can be very tricky. I agree this should be removed.

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 a pull request may close this issue.

3 participants