Improve performance with NPM package

[nesk]

Using lefthook through the NPM package causes performance issues, because NPX is a bit slow to start and execute each time lefthook is invoked. This can especially being seen when doing rebases with dozens of commits.

Here's a basic benchmark between native binary and NPM:

# Native:
$ time lefthook version
0.7.2
lefthook version  0.01s user 0.01s system 85% cpu 0.014 total

# NPM:
$ time npx lefthook version
0.7.2
npx lefthook version  0.09s user 0.04s system 97% cpu 0.125 total

If you rebase some commits, it will take ~9 times longer to finish rebasing with NPM vs native binary.

This PR makes the hooks search for the final binaries by themselves, rather than using the npx command (which is now only used as a fallback).

Here's a benchmark comparing native binary, NPM before this PR, and NPM after this PR:

# Native:
$ time git rebase -i HEAD~3
Successfully rebased and updated refs/heads/my-branch-name.
git rebase -i HEAD~3  0.28s user 0.25s system 24% cpu 2.138 total

# NPM before this PR:
$ time git rebase -i HEAD~3
Successfully rebased and updated refs/heads/my-branch-name.
git rebase -i HEAD~3  2.50s user 0.95s system 70% cpu 4.887 total

# NPM after this PR:
$ time git rebase -i HEAD~3
Successfully rebased and updated refs/heads/my-branch-name.
git rebase -i HEAD~3  0.27s user 0.26s system 34% cpu 1.559 total

You can see this PR makes NPM as fast as the native binary.

[nesk]

You can see we're checking if the binaries in the NPM packages exists before the Ruby part. This seems a bit odd at the first glance… but it improves performance for NPM packages because running bundle exec is quite slow. And adding a simple file existence check before the Ruby part is not an issue: the check is fast enough to not penalize Ruby users.

[Arkweid]

Great work! Will check it soon.

[Arkweid]

@nesk
I've got a series of errors when testing file bin/lefthook

./lefthook: Bad substitution
./lefthook: [[: not found
./lefthook: [[: not found
./lefthook: [[: not found
./lefthook: [[: not found
./lefthook: [[: not found
./lefthook: Unsupported: not found

Could you look at?
It works when I switch sh to bash except Unsupported OS, there should be echo "Unsupported OS"

[nesk]

Sorry, I should have been more careful. Indeed, we should switch to bash instead of sh because it was written with Bash in mind, and Unsupported OS should outputted so we should use echo "Unsupported OS".

Do you want a PR for this?

[Arkweid]

@nesk could we switch syntax to sh like in other scripts? I think in some systems bash is not supported(some docker environments/windows). Is it possible?

[nesk]

After a bit of searching, unfortunately sh is not a specific command line interpreter, its whatever the system wants to provide us with.

I think we have two ways to manage this:

• We add a bash requirement.
• We try to improve the script, shouldn't be too hard but we will need to know how to reproduce the problem. Could you describe me the environment to reproduce what you posted earlier?

I'm really sorry I messed up your codebase… 😞

[Arkweid]

@nesk Here a docker file for Discourse. I test new builds for this one.

# TEST DISCOURSE LOCALY
# sudo docker build -t discourse-lefthook-test .
# sudo docker run -it discourse-lefthook-test bash

FROM ruby:2.6.6-slim-buster

# Common dependencies
RUN apt-get update -qq \
  && DEBIAN_FRONTEND=noninteractive apt-get install -yq --no-install-recommends \
    build-essential \
    gnupg2 \
    curl \
    git \
  && apt-get clean \
  && rm -rf /var/cache/apt/archives/* \
  && rm -rf /var/lib/apt/lists/* /tmp/* /var/tmp/* \
  && truncate -s 0 /var/log/*log

# Add NodeJS to sources list
RUN curl -sL https://deb.nodesource.com/setup_12.x | bash -

# Add Yarn to the sources list
RUN curl -sS https://dl.yarnpkg.com/debian/pubkey.gpg | apt-key add - \
  && echo 'deb http://dl.yarnpkg.com/debian/ stable main' > /etc/apt/sources.list.d/yarn.list

RUN apt-get update -qq && DEBIAN_FRONTEND=noninteractive apt-get -yq dist-upgrade && \
  DEBIAN_FRONTEND=noninteractive apt-get install -yq --no-install-recommends \
    libpq-dev \
    nodejs \
    yarn

RUN git clone https://github.com/discourse/discourse.git --depth=1

WORKDIR /discourse

RUN gem update --system && \
    gem install bundler

RUN bundle install
RUN yarn install

# COPY lefthook .

Now apply changes for npm package:

cd /discourse/node_modules/@arkweid/lefthook/bin
// rewrite indes.js and create lefthook file with script. Don't forget to make it executable
chmod +x lefthook 

// Now return to discourse/ dir 
cd -
yarn lefthook run lints

// And got this:
root@a86cc5b35f62:/discourse# yarn lefthook run lints
yarn run v1.22.4
$ /discourse/node_modules/.bin/lefthook run lints
/discourse/node_modules/@arkweid/lefthook/bin/lefthook: 3: /discourse/node_modules/@arkweid/lefthook/bin/lefthook: Bad substitution
/discourse/node_modules/@arkweid/lefthook/bin/lefthook: 5: /discourse/node_modules/@arkweid/lefthook/bin/lefthook: [[: not found
/discourse/node_modules/@arkweid/lefthook/bin/lefthook: 7: /discourse/node_modules/@arkweid/lefthook/bin/lefthook: [[: not found
/discourse/node_modules/@arkweid/lefthook/bin/lefthook: 9: /discourse/node_modules/@arkweid/lefthook/bin/lefthook: [[: not found
/discourse/node_modules/@arkweid/lefthook/bin/lefthook: 9: /discourse/node_modules/@arkweid/lefthook/bin/lefthook: [[: not found
/discourse/node_modules/@arkweid/lefthook/bin/lefthook: 9: /discourse/node_modules/@arkweid/lefthook/bin/lefthook: [[: not found
/discourse/node_modules/@arkweid/lefthook/bin/lefthook: 12: /discourse/node_modules/@arkweid/lefthook/bin/lefthook: Unsupported: not found

[nesk]

See #154 for a bugfix 🙂