Binwrap slows execution down

[andys8]

My editor is executing elm-format a lot. It looks like binwrap is slowing down execution. Same effect with npm only. I think we need more comparisons on other systems and if binwrap is the cause we should look into something like the elm "no-deps" installer, which is using npm but looks like it isn't slowing execution down on my machine.

https://github.com/elm/compiler/tree/master/installers/npm

450ms vs 80ms

which -a elm-format                                                                                                                                
/tmp/fnm-shell-3625665/bin/elm-format
/home/as/bin/elm-format
​
​
time /tmp/fnm-shell-3625665/bin/elm-format src/Main.elm --yes                                                                                      
Processing file src/Main.elm
0.48user 0.04system 0:00.45elapsed 114%CPU (0avgtext+0avgdata 52876maxresident)k
0inputs+0outputs (0major+9793minor)pagefaults 0swaps
​
time /home/as/bin/elm-format src/Main.elm --yes                                                                                                    
Processing file src/Main.elm
0.07user 0.00system 0:00.08elapsed 89%CPU (0avgtext+0avgdata 14856maxresident)k
0inputs+0outputs (0major+1608minor)pagefaults 0swaps

[BendingBender]

Similar results here:

422ms vs 49ms

$ ls -la $(which elm-format)
lrwxr-xr-x  1 db  staff  45 Jun 21 14:26 /Users/db/.nvm/versions/node/v10.16.0/bin/elm-format -> ../lib/node_modules/elm-format/bin/elm-format

$ time /Users/db/.nvm/versions/node/v10.16.0/lib/node_modules/elm-format/bin/elm-format src/Main.elm --yes

real	0m0.422s
user	0m0.368s
sys		0m0.071s

$ time /Users/db/.nvm/versions/node/v10.16.0/lib/node_modules/elm-format/unpacked_bin/elm-format src/Main.elm --yes
Processing file src/Main.elm

real	0m0.049s
user	0m0.030s
sys		0m0.010s

[avh4]

Is the overhead mostly in starting node, or in something the js code is doing?

[andys8]

Added some time measurements:

Summary

• Node upstart: ~60ms
• Imports in node script (require): 187ms
• Script running until spawn: 2ms
• Spawn with elm-format: 11ms (??)

Note: Quick measurements, please verify with your own. Also this is a different machine, than my previous post.

Logs

time elm-format                                                                                                        
import: 187.111ms
total: 189.114ms
elm-format 0.8.1

Usage: elm-format [INPUT] [--output FILE] [--yes] [--validate] [--stdin]
                  [--elm-version VERSION] [--upgrade]
  Format Elm source files.

Available options:
  -h,--help                Show this help text
  --output FILE            Write output to FILE instead of overwriting the given
                           source file.
  --yes                    Reply 'yes' to all automated prompts.
  --validate               Check if files are formatted without changing them.
  --stdin                  Read from stdin, output to stdout.
  --elm-version VERSION    The Elm version of the source files being formatted.
                           Valid values: 0.18, 0.19. Default: auto
  --upgrade                Upgrade older Elm files to Elm 0.19 syntax

Examples:
  elm-format Main.elm                     # formats Main.elm
  elm-format Main.elm --output Main2.elm  # formats Main.elm as Main2.elm
  elm-format src/                         # format all *.elm files in the src directory

Full guide to using elm-format at <https://github.com/avh4/elm-format>
0.30user 0.01system 0:00.26elapsed 117%CPU (0avgtext+0avgdata 51856maxresident)k
0inputs+0outputs (0major+10958minor)pagefaults 0swaps

Node only

time node -e "console.log('does nothing')"                                                                             
does nothing
0.04user 0.01system 0:00.06elapsed 100%CPU (0avgtext+0avgdata 30180maxresident)k
0inputs+0outputs (0major+2231minor)pagefaults 0swaps

[andys8]

Looks like moving those lines:

  var packageInfo = require(path.join(__dirname, "..", "package.json"));
  var package = require(path.join(__dirname, "..", packageInfo.main));

in the block where they're needed, will improve from 260ms to 80ms.

Full file

#!/usr/bin/env node
console.time("total")
console.time("import")
var path = require("path");
var spawn = require("child_process").spawn;
var fs = require("fs");
console.timeEnd("import")

var os = process.env.BINWRAP_PLATFORM || process.platform;
var arch = process.env.BINWRAP_ARCH || process.arch;

var requested = os + "-" + arch;
var current = process.platform + "-" + process.arch;
if (requested !== current ) {
  console.error("WARNING: Using binaries for the requested platform (" + requested + ") instead of for the actual platform (" + current + ").")
}

var binExt = "";
if (os == "win32") {
  binExt = ".exe";
}

var unpackedBinPath = path.join(__dirname, "..", "unpacked_bin");
var binPath = path.join(unpackedBinPath, "elm-format" + binExt);

function execBin() {
  console.timeEnd("total")
  spawn(
    binPath,
    process.argv.slice(2),
    {stdio: 'inherit'}
  ).on('exit', process.exit);
}

if (fs.existsSync(binPath)) {
  execBin();
} else {
  console.error("INFO: Running " + path.basename(__filename) + " for the first time; downloading the actual binary");
  var packageInfo = require(path.join(__dirname, "..", "package.json"));
  var package = require(path.join(__dirname, "..", packageInfo.main));
  package.install(unpackedBinPath, os, arch).then(function(result) {
    execBin();
  }, function(err) {
    console.log("ERR", err);
    process.exit(1);
  });
}

Comparison

For reference: Using the binary directly takes 10ms. So it would still make sense to eliminate the node script for execution.

time /home/user/bin/elm-format
elm-format 0.8.1

Usage: elm-format [INPUT] [--output FILE] [--yes] [--validate] [--stdin]
                  [--elm-version VERSION] [--upgrade]
  Format Elm source files.

Available options:
  -h,--help                Show this help text
  --output FILE            Write output to FILE instead of overwriting the given
                           source file.
  --yes                    Reply 'yes' to all automated prompts.
  --validate               Check if files are formatted without changing them.
  --stdin                  Read from stdin, output to stdout.
  --elm-version VERSION    The Elm version of the source files being formatted.
                           Valid values: 0.18, 0.19. Default: auto
  --upgrade                Upgrade older Elm files to Elm 0.19 syntax

Examples:
  elm-format Main.elm                     # formats Main.elm
  elm-format Main.elm --output Main2.elm  # formats Main.elm as Main2.elm
  elm-format src/                         # format all *.elm files in the src directory

Full guide to using elm-format at <https://github.com/avh4/elm-format>
0.00user 0.00system 0:00.01elapsed 57%CPU (0avgtext+0avgdata 6444maxresident)k
192inputs+0outputs (0major+356minor)pagefaults 0swaps

[razzeee]

So is what @andys8 tracked down something we can do or would that cause problems?
Or can we create a binwrap free version like the elm compiler is now?

Unfortunatly I could not find any docs, how @evancz did that for the compiler.

[andys8]

As far as I know, the trick is/was to overwrite the node file with an actual binary. But there is a recent commit, and I'm not sure, how the comment is meant. It looks like it's reverting the change.

elm/compiler@33e43e8#diff-fa7bb253843eb34c43642a2324c06571

[TimonVS]

Same issue here, I'm pretty sure it used to work fine on my old Mac though 🤔 .

elm-format installed through npm (0.713s):

λ ~/Projects/personal/elm-in-action/PhotoGroove/ which elm-format
/Users/timon/.asdf/installs/nodejs/10.16.0/.npm/bin/elm-format
λ ~/Projects/personal/elm-in-action/PhotoGroove/ time elm-format
elm-format 0.8.1

Usage: elm-format [INPUT] [--output FILE] [--yes] [--validate] [--stdin]
                  [--elm-version VERSION] [--upgrade]
  Format Elm source files.

Available options:
  -h,--help                Show this help text
  --output FILE            Write output to FILE instead of overwriting the given
                           source file.
  --yes                    Reply 'yes' to all automated prompts.
  --validate               Check if files are formatted without changing them.
  --stdin                  Read from stdin, output to stdout.
  --elm-version VERSION    The Elm version of the source files being formatted.
                           Valid values: 0.18, 0.19. Default: auto
  --upgrade                Upgrade older Elm files to Elm 0.19 syntax

Examples:
  elm-format Main.elm                     # formats Main.elm
  elm-format Main.elm --output Main2.elm  # formats Main.elm as Main2.elm
  elm-format src/                         # format all *.elm files in the src directory

Full guide to using elm-format at <https://github.com/avh4/elm-format>
elm-format  0.42s user 0.20s system 87% cpu 0.713 total

elm-format from binary (0.020s):

λ ~/Projects/personal/elm-in-action/PhotoGroove/ time ~/bin/elm-format 
elm-format 0.8.1

Usage: elm-format [INPUT] [--output FILE] [--yes] [--validate] [--stdin]
                  [--elm-version VERSION] [--upgrade]
  Format Elm source files.

Available options:
  -h,--help                Show this help text
  --output FILE            Write output to FILE instead of overwriting the given
                           source file.
  --yes                    Reply 'yes' to all automated prompts.
  --validate               Check if files are formatted without changing them.
  --stdin                  Read from stdin, output to stdout.
  --elm-version VERSION    The Elm version of the source files being formatted.
                           Valid values: 0.18, 0.19. Default: auto
  --upgrade                Upgrade older Elm files to Elm 0.19 syntax

Examples:
  elm-format Main.elm                     # formats Main.elm
  elm-format Main.elm --output Main2.elm  # formats Main.elm as Main2.elm
  elm-format src/                         # format all *.elm files in the src directory

Full guide to using elm-format at <https://github.com/avh4/elm-format>
~/bin/elm-format  0.00s user 0.00s system 37% cpu 0.020 total

On my old Mac I used n to install Node, on my current Mac I'm using asdf with the Node.js plugin. Though this probably isn't relevant since other people are having the same problem with slowness.

[BendingBender]

I've just asked Evan about the revert, here's our coversation:

Me:

Hi Evan, sorry for contacting you directly, I just wasn’t sure where to post this.
There is an issue on github with the binwrap (#31) package being slow to execute the elm-format binary. There was some discussion on doing the same as you did for the node installer for the Elm compiler, namely replacing the JS file with the binary upon the first run. But we saw that you’ve reverted this change in elm/compiler@33e43e8#diff-fa7bb253843eb34c43642a2324c06571.
I just wanted to ask you on the reasons why you’ve reverted this, your comit message is a bit cryptic.

Evan:

It seems that on Windows the 0.19.0-no-deps tries to download the binary every time for people who have the --ignore-scripts flag enabled.
So the code you are seeing there is an attempt to avoid that.
The problem is that the binary is called elm.exe on Windows, so it doesn't actually overwrite the JS file as we'd prefer.
I think we'd need to give the JS file a name like bin/elm.cmd to make sure it works on Windows, but then it looks pretty weird on Linux and Mac. It should work there, but it's still a bit odd!
Does that make sense? (I'd like to find an ideal solution for overwriting the JS file, so I'm happy to chat about it!)

[avh4]

I made the change suggested by @andys8 here: #33

I believe a much older version of binwrap did overwrite the wrapper with the downloaded binaries, but that turned out to have issues both on Windows (as noted above), and on other systems (I think mainly some linuxes) where users did not have permissions to overwrite the files.

I'd be open to a solution that does overwrite, but it'd need to be thoroughly tested on different system environments and have a safe fallback on systems where the overwrite doesn't work or isn't safe.

[avh4]

Also, thanks everyone for the helpful details, and especially @andys8 for the detailed investigation.

[andys8]

Cool. What are your ideas about this topic? An additional improvement could be to replace the script with the binary only on mac and unix. This would improve the execution for a large percentage, at the cost of introduction slightly different behavior. I'd also prefer to target a single working approach and use it with elm itself, too. It's solving the same problem.

But also: Feel free to close this issue :)

[BendingBender]

I've just investigated a bit what a global installation of elm-format looks like on Windows. It seems that npm on Windows does the following:

1. it installs the package into %USERPROFILE%\AppData\Roaming\npm\node_modules
2. it creates 2 scripts in %USERPROFILE%\AppData\Roaming\npm:
   • elm-format which is a bash script that explicitly calls node with the path of the elm-format bin (%USERPROFILE%\AppData\Roaming\npm\node_modules\elm-format\bin\elm-format)
   • elm-format.cmd which is a Windows batch script that does basically the same, namely calling the elm-format bin with node explicitly

So this all means that we can't really replace the node script with the actual binary on Windows because the bin script is always being called with node explicitly. From my perspective there are 2 options here:

• to leave Windows as is, the binary will be executed through node
• to replace the scripts in %USERPROFILE%\AppData\Roaming\npm with the actual binary, although in my opinion we shouldn't do anything that depends upon the inner workings of npm