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 performance greatly #337
Conversation
|
Originally, api allowing multiple arguments was killing performance quite a lot, but with moving it out of hot path and providing fast-path for single argument it doesn't seem to be an issue anymore, I didn't look at JIT info, but my guess would be with root function being so small it just gets inlined most times. |
Another insight is that for full benchmark, time is spent:
Which shows us that:
|
Plus, another important thing is that not about benchmark framework. First of all, it doesn't perform dry-run, so on your normal PC it first benches show weird results in any case. Second, it seem to make this mistake I mentioned from the beginning with just plain incorrect calculation of first bench sometimes. But having played with manual dry runs, I can say that performance difference between cached and non-cached versions for simple styling(complex styles like RGB aren't cached at all at the moment), is usually <2%, so for most use cases caching won't be needed at all anymore. |
Providing shorthands like |
Not sure whether you mean internally or externally. My intention was to just optimize for common use-cases internally, not to expose properties like |
Yes, from experience, V8 is not very good at optimizing complicated prototypes. |
This is really awesome! 🙌 Are there any more optimization opportunities you can think of? Or are you done? |
Can you give the PR a proper descriptive title? |
Really wish I had @RemindMe running. I want to devote some time to this PR. |
I also want to incorporate require speed improvements in this patch, which will should also allow to get some runtime speed improvements. It was sitting in the back of my mind, and I don't recall all my iterations and if I ruled it out, but I'm already considering writing a github parser to extract common use cases, but I'm not sure I will get to it. There is a lot of questions, but one really important thing is if people frequently use nested styles as in running Also, I've tried to add things incrementally with checks at every step to not ruin anything, but I don't completely understand how |
Can you do that in a follow-up PR? This PR is already large and has been open for a long time. I would like to be able to merge this soon. |
So what's realistically left to do here? |
@sindresorhus Yeah, I will leave other changes to separate PR, the core changes seem to be there already, so should be easy enough to extend on them, plus it's a huge step as it is already. Rebased, fixed style, moved utils to separate file. |
@stroncium This looks great. I really appreciate your work on this 🙌 Regarding the require speed improvements, do you think you'll be able to get to that within a couple of weeks? I'm asking as I would like to get out Chalk 3 soon and wondering whether or not I should wait for the require speed improvements. |
@sindresorhus I reckon I should be able to. Just got a lot of work in last weeks plus procfs lib documentation writing proved to be a bit bigger task than anticipated(at 20-40 pages of documentation atm depending on how you look at it lol), but the latter is almost finished and I actually hope to make major release today or tomorrow. After that, I'm getting back to my backlog |
Ok. Hehe. No worries. |
@stroncium FYI, it's finally out: https://twitter.com/sindresorhus/status/1193065505780727808 Thanks again for all your work on the performance improvements. 👌 |
@stroncium Looking at this a year later, something took my attention. 😄 You have written |
That's a blanket statement and not very constructive. Linked lists are useful and can very much be performant depending on how they're used. Otherwise, nobody would use them had they no use. If you have a criticism of a specific piece of code, please go into detail. Pointing someone to an absolutist video about data structures when clearly the OP knows a bit of what they're doing can be seen as condescending. Further, it doesn't provoke conversation. |
@aminya Apart from what have been said, I'm really curios why did you place this critics here seemingly out of random when there are whole languages where there is no arrays of any kind at all traditionally(hey Python, hey Lisp!). Anyway, going back to the very video you posted, I'd advise you to watch it first, because it alone describes half of why linked lists are appliable in this case in general. But speaking about programming in javascript specifically, linked list argument is also invalid in most other cases as long as you use any kind of objects(which are objects as in pointers to vtable headed structures and not structures itself). |
Fixes #333
Benchmark:
Based on my own usage of chalk, I select KPM:
cached
: average ofcached: 1 style
andcached: 2 styles
metricsnon-cached
: average of1 style
and2 styles
metricstotal
: average of all metricsKPM increase:
cached
: x23non-cached
: x129total
: x24Changes(incrementally increasing preformance):
this
)Original(with new benchmark) metrics:
Original KPM:
cached
: 1078 kop/snon-cached
: 200 kop/stotal
: 794 kop/sResult metrics:
Result KPM:
cached:
24925 kop/snon-cached
: 25820 kop/stotal
: 19145 kop/s