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

Performance optimization #376

Closed
cowwoc opened this issue Sep 30, 2013 · 14 comments
Closed

Performance optimization #376

cowwoc opened this issue Sep 30, 2013 · 14 comments

Comments

@cowwoc
Copy link

cowwoc commented Sep 30, 2013

I love the API design, but sugar.js tends to have one of the worse performance results when compared to other libraries. Please take a look at http://danieltao.com/lazy.js/comparisons.html.

Is it possible to borrow any of their techniques to improve sugar.js's performance? See http://danieltao.com/lazy.js/index.html

@andrewplummer
Copy link
Owner

I understand where you're coming from and Sugar has been placing more importance on performance recently, but it's important to make sure we're comparing apples to apples when looking at this.

First off, I noticed that the page referenced is using an older version of Sugar taken from the edge version a while back. From the look of it it's most likely 1.3.9 and in fact there was a major performance refactor that took place in v1.4.

Another point is that for some of the methods listed there Sugar falls through to native implementations so what you're looking at may not in fact be Sugar from the start. I know that both Backbone and lo-dash often have more straightforward implementations of their native-equivalents that they can get away with because they're not polyfilling and so do not have to match all of the more edge-casy paths that are required by the spec which can have a performance hit. As Sugar is extending natives it is by definition polyfilling which means it has to follow spec.

Another point is that in many cases Sugar is simply doing more than the others here. One example of this is Sugar's version of map which falls back to native implementations but is also capable of understanding strings a'la Ruby et.al. There is a very simple check done up front but even this takes its toll over millions of iterations. Performance at this point is now a first-class citizen in Sugar but it still does not trump the added convenience that these methods provide, and has to be balanced into the overall equation.

Finally, lazy.js is very interesting but it's an entirely different approach that doesn't map to the other libraries listed in a black-and-white manner (it even says that on their site). I think it's pretty clear that it isn't intended to be a replacement for low level utilities libraries, but instead its own library that can be added in when massive arrays need to be quickly traversed (most commonly while chaining methods).

The bottom line is that while there are always more places to tighten down and get performance gains, as of v1.4 (this was not true previous to this version) it is starting to reach an equilibrium where performance gains have diminishing returns.

One concrete example of this is the "function inlining" route that lo-dash has gone. I had this on the radar to try for a long time, and made an attempt as part of the final refactoring of v1.4. However it quickly got out of hand and I didn't see as big a performance increase as I had hoped for (nothing compared to the other relatively simple refactorings I had done), and in the end I had to scrap it.

Anyway I'm always open to concrete suggestions of places that could be tightened down but after just recently sweeping through the entire codebase doing just that, I'm fairly happy in the balance that Sugar has finally struck.

@cowwoc
Copy link
Author

cowwoc commented Oct 1, 2013

@andrewplummer I tried (and failed) to find benchmarks from anyone that compared newer Sugar versions to the competition. Perhaps now is a good time to put one up on http://jsperf.com/ and point to it from the project page?

I agree with you that we should favor convenience and good design over performance, but we should take a more active interest in:

  1. Tracking performance over time (you can't improve what you don't measure).
  2. Ensuring the performance hit is reasonable for the benefits we are adding. Meaning, we might be able to justify a 2x-10x performance difference, but not 100x, given our current feature set.

@andrewplummer
Copy link
Owner

  1. Sounds reasonable
  2. This really all depends. Too many people take relative numbers into account without taking any notice of the absolute. But in general I agree here that there is a balance out there and somewhere there is a performance hit that is "too big"

@iabdulin
Copy link

Date formatting:
http://jsperf.com/sugar-js-dates-vs-moment-js

image

@andrewplummer
Copy link
Owner

Thanks for posting this!

FWIW I just ran it and this is what I got:

screen shot 2014-04-28 at 4 06 43 am

A few more re-tries shows they're really neck and neck.

At any rate I'll keep this link and refer back to it when making optimizations and improvements and make sure that new features don't open up a huge performance gap (as was the case wither earlier versions).

Thanks!

@andrewplummer
Copy link
Owner

By the way, as a point of reply to @cowwoc, this particular test is a good example of a beneficial test to set up. This is because the format method is doing something straightforward and in that sense it can be meaningfully compared to moment.js... However the point here is that that may not always be the case, even if the methods being compared have the same name, their feature set may vary enough to make it an apples to oranges comparison, in which case I don't find making jsperf test cases to be beneficial.

@iabdulin
Copy link

People performance is more important than js performance most of the times :)
Who cares if it's 100,000 ops/sec or 10,000 ops/sec. Nice human readable API and compatibility far outweight raw performance.

Of course, there're edge cases when performance is really important, but if you focus on 95% usecases for Sugar.js, then 95% users will be happy.

@iabdulin
Copy link

Here's another test, but I am not sure that it has any use.
At least, you can change it to do smth useful instead :)

http://jsperf.com/sugar-js-array-vs-native-vs-lodash-js

@andrewplummer
Copy link
Owner

A couple interesting things I noted on that one.

Sugar was actually coming out on top on iOS7. What's odd though is that it was beating out the native for loop, which almost certainly can't be because it USES a native for loop of course PLUS a hasOwnProperty check, so I'm not sure what to make of that one...

As a sanity check to see if it wasn't simply erroring I set up an experiment to see if Object.each was functioning properly but it appears to be...

In desktop Chrome Lodash also comes out on the bottom, which also seems odd to me as Lodash typically has outstanding performance.

Anyway thanks for the test!

@iabdulin
Copy link

@andrewplummer don't trust this test, I am not sure that it is valid.
About beating native loop: jsperf displays inaccuracy level on each test, I bet if you take those into account, the performance will be the same.

@andrewplummer
Copy link
Owner

Where is the accuracy displayed?

@iabdulin
Copy link

201,573 +/– 3.47%
185,367 +/– 1.57%

So, in worst case inaccuracy will be 3.47+1.57 = 5.04%

image

@andrewplummer
Copy link
Owner

agh I'm blind ... it's right there :)

@andrewplummer andrewplummer added Docs and removed Docs labels Apr 29, 2014
@andrewplummer
Copy link
Owner

Going to close this one out for now as an open issue.
Will be keeping my eye on it, however. If specific performance issues crop up that can be tangibly addressed, I will turn them into new actionable issues.

Also I will start to think about how/where to keep a performance log to make sure there isn't any performance regression.

Thanks!

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

3 participants