brace-expansion
brace-expansion copied to clipboard
Improvements
add some (hopefully) useful comments
don't use regex when possible
combine some if statements
remove unnecessary isTop
check
this PR increases about 15 op/s there is no logic changes
@juliangruber can you enable travis for this module too? 😄 Hmm... looks like its not building on PRs.
(idk why it was disabled but it's enabled again)
@isaacs seems good to you?
i'm still in the process of reviewing the "general improvements" commit, just want to make sure this doesn't change anything that isn't covered by tests.
this module is being used a lot, so improving it for 15 ops/s maybe isn't the best idea, but I don't want to turn down a good effort either so let's keep evaluating.
No problem. I'm sure there are other places to improve in the code but I'd like to start from here
The main purpose of this is not too much of perf improvements. More just refactoring.
On 22 Jul 2016 7:12 AM, "Julian Gruber" [email protected] wrote:
i'm still in the process of reviewing the "general improvements" commit, just want to make sure this doesn't change anything that isn't covered by tests.
this module is being used a lot, so improving it for 15 ops/s maybe isn't the best idea, but I don't want to turn down a good effort either so let's keep evaluating.
— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/juliangruber/brace-expansion/pull/28#issuecomment-234385598, or mute the thread https://github.com/notifications/unsubscribe-auth/AGBiLnj3fCX43xCNAkVoWfqzWhMWuAIdks5qX-C5gaJpZM4JRwXT .
Using the cache and O(log(n)) algorithm for repeating 0
increases the performance quite good! On my machine, it increases from 460 op/s to 530 op/s (numbers are approximate).