search

imagemin / cwebp-bin

cwebp bin-wrapper that makes it seamlessly available as a local dependency
https://developers.google.com/speed/webp
MIT License
93 stars 36 forks source link

Do not load any dependencies in production #6

Closed shinnn closed 9 years ago

shinnn commented 9 years ago

This PR prevents cwebp-bin from loading any dependencies when it is used as a module. When cwebp-bin is required, it won't use any external modules such as bin-wrapper.

Current workflow

  1. bin-wrapper specifies the binary path with bin.use.
  2. index.js refers to the path.

    New workflow

  3. index.js specifies the binary path.
  4. Only when cwebp-bin is installed from npm, it uses bin-wrapper. bin-wrapper refers to index.js for the destination of installation.

After merging this PR, imagemin plugins won't load large dependencies in production. It helps all imagemin users to save grunt/gulp build time, especially in starting the tasks.

@imagemin/owners

Though this PR doesn't break API, it includes major changes in directory structure and code. I'd like you to review it.

kevva commented 9 years ago

Why merge?

shinnn commented 9 years ago

@kevva Sorry, I jumped to a wrong conclusion that this PR was silently approved. I'll revert or fix it soon if you have a better idea.

shinnn commented 9 years ago

Here is the result of time-require.

script

'use strict';

require('time-require');
require("cwebp-bin");

output

with cwebp-bin v2.0.4 (new version)

Start time: (2015-01-26 19:55:48 UTC) [treshold=1%]
#  module                          time  %
1  text-table (no...ble/index.js)   1ms  ▇▇▇ 8%
2  date-time (nod...ime/index.js)   2ms  ▇▇▇▇▇ 17%
3  parse-ms (node...-ms/index.js)   1ms  ▇▇▇ 8%
4  pretty-ms (nod...-ms/index.js)   2ms  ▇▇▇▇▇ 17%
5  ansi-styles (n...si-styles.js)   1ms  ▇▇▇ 8%
6  has-color (nod...lor/index.js)   1ms  ▇▇▇ 8%
7  chalk (node_mo...alk/index.js)   4ms  ▇▇▇▇▇▇▇▇▇▇ 33%
8  cwebp-bin (nod...bin/index.js)   1ms  ▇▇▇ 8%
Total require(): 10
Total time: 12ms

with cwebp-bin v2.0.2 (old version)

Start time: (2015-01-26 19:57:59 UTC) [treshold=1%]
 #  module                           time  %
 1  chalk (node_mo...alk/index.js)    5ms  ▇ 2%
 2  child_process...ble/index.js)     3ms  ▇ 1%
 3  bin-check (nod...eck/index.js)    5ms  ▇ 2%
 4  find-versions...ons/index.js)     3ms  ▇ 1%
 5  bin-version (n...ion/index.js)    4ms  ▇ 2%
 6  bin-version-ch...eck/index.js)    8ms  ▇ 3%
 7  ./lib/_stream_..._readable.js)    6ms  ▇ 2%
 8  readable-strea.../readable.js)   11ms  ▇▇ 4%
 9  typedarray (no...ray/index.js)    5ms  ▇ 2%
10  concat-stream...eam/index.js)    17ms  ▇▇ 6%
11  rc (node_modul.../rc/index.js)    4ms  ▇ 2%
12  vinyl (node_mo...nyl/index.js)    4ms  ▇ 2%
13  ./_stream_dupl...am_duplex.js)    6ms  ▇ 2%
14  ./lib/_stream_...transform.js)    6ms  ▇ 2%
15  readable-strea...transform.js)    7ms  ▇ 3%
16  through2 (node.../through2.js)    9ms  ▇▇ 3%
17  brace-expansio...ion/index.js)    4ms  ▇ 2%
18  minimatch (nod...minimatch.js)    5ms  ▇ 2%
19  glob (node_mod...glob/glob.js)   13ms  ▇▇ 5%
20  glob-stream (n...eam/index.js)   17ms  ▇▇ 6%
21  ./fs.js (node_...ful-fs/fs.js)    4ms  ▇ 2%
22  graceful-fs (n...aceful-fs.js)    9ms  ▇▇ 3%
23  ./bufferFile (...ufferFile.js)   12ms  ▇▇ 5%
24  ./getContents...nts/index.js)    13ms  ▇▇ 5%
25  ./lib/src (nod...src/index.js)   43ms  ▇▇▇▇▇ 16%
26  ./writeContent...nts/index.js)    4ms  ▇ 2%
27  ./lib/dest (no...est/index.js)    7ms  ▇ 3%
28  lodash (node_m...st/lodash.js)    9ms  ▇▇ 3%
29  graceful-fs (n...aceful-fs.js)    3ms  ▇ 1%
30  minimatch (nod...minimatch.js)    4ms  ▇ 2%
31  glob (node_mod...glob/glob.js)   10ms  ▇▇ 4%
32  globule (node_...b/globule.js)   20ms  ▇▇▇ 8%
33  gaze (node_mod.../lib/gaze.js)   22ms  ▇▇▇ 8%
34  glob-watcher (...her/index.js)   22ms  ▇▇▇ 8%
35  vinyl-fs (node...-fs/index.js)   73ms  ▇▇▇▇▇▇▇▇▇ 27%
36  ./lib/_stream_...am_duplex.js)    6ms  ▇ 2%
37  readable-strea...am/duplex.js)    6ms  ▇ 2%
38  bl (node_modul...les/bl/bl.js)    7ms  ▇ 3%
39  ./extract (nod...m/extract.js)   12ms  ▇▇ 5%
40  ./pack (node_m...ream/pack.js)    3ms  ▇ 1%
41  tar-stream (no...eam/index.js)   15ms  ▇▇ 6%
42  decompress-tar...tar/index.js)   20ms  ▇▇▇ 8%
43  seek-bzip (nod...zip/index.js)    4ms  ▇ 2%
44  ./_stream_read..._readable.js)    3ms  ▇ 1%
45  ./lib/_stream_...am_duplex.js)    7ms  ▇ 3%
46  readable-strea...am/duplex.js)    8ms  ▇ 3%
47  bl (node_modul...les/bl/bl.js)    8ms  ▇ 3%
48  readable-strea.../readable.js)    3ms  ▇ 1%
49  ./extract (nod...m/extract.js)   13ms  ▇▇ 5%
50  ./pack (node_m...ream/pack.js)    3ms  ▇ 1%
51  tar-stream (no...eam/index.js)   16ms  ▇▇ 6%
52  decompress-tar...bz2/index.js)   23ms  ▇▇▇ 9%
53  ./_stream_read..._readable.js)    3ms  ▇ 1%
54  ./lib/_stream_...am_duplex.js)    6ms  ▇ 2%
55  readable-strea...am/duplex.js)    7ms  ▇ 3%
56  bl (node_modul...les/bl/bl.js)    7ms  ▇ 3%
57  ./extract (nod...m/extract.js)   11ms  ▇▇ 4%
58  ./pack (node_m...ream/pack.js)    3ms  ▇ 1%
59  tar-stream (no...eam/index.js)   14ms  ▇▇ 5%
60  zlib (node_mod...eam/index.js)    3ms  ▇ 1%
61  decompress-tar...rgz/index.js)   22ms  ▇▇▇ 8%
62  ./fs.js (node_...ful-fs/fs.js)    3ms  ▇ 1%
63  graceful-fs (n...aceful-fs.js)    5ms  ▇ 2%
64  crypto (crypto)                   4ms  ▇ 2%
65  ./rng (node_mo.../uuid/rng.js)    5ms  ▇ 2%
66  uuid (node_mod...uuid/uuid.js)    5ms  ▇ 2%
67  temp-write (no...ite/index.js)   12ms  ▇▇ 5%
68  ./util (node_m...til/index.js)    3ms  ▇ 1%
69  ./methods (nod...ods/index.js)    4ms  ▇ 2%
70  ./zipEntry (no.../zipEntry.js)   10ms  ▇▇ 4%
71  adm-zip (node_...p/adm-zip.js)   11ms  ▇▇ 4%
72  decompress-unz...zip/index.js)   28ms  ▇▇▇▇ 11%
73  download (node...oad/index.js)  204ms  ▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇ 77%
74  minimatch (nod...minimatch.js)    4ms  ▇ 2%
75  glob (node_mod...glob/glob.js)    9ms  ▇▇ 3%
76  globby (node_m...bby/index.js)   12ms  ▇▇ 5%
77  chalk (node_mo...alk/index.js)    5ms  ▇ 2%
78  download-statu...tus/index.js)    8ms  ▇ 3%
79  rc (node_modul.../rc/index.js)    4ms  ▇ 2%
80  npm-which (nod...ich/index.js)    3ms  ▇ 1%
81  npm-installed...led/index.js)    12ms  ▇▇ 5%
82  bin-wrapper (n...per/index.js)  255ms  ▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇ 96%
83  ./lib (node_mo...lib/index.js)  256ms  ▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇ 96%
84  cwebp-bin (nod...bin/index.js)  257ms  ▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇ 97%
Total require(): 566
Total time: 266ms
sindresorhus commented 9 years ago

Yeah, I agree something should be done.

shinnn commented 9 years ago

As the benchmark above said, the newer version is 20 times faster than ever.

If we reach a consensus here, I'd like to update all the other *-bin modules like this PR.

kevva commented 9 years ago

Yeah, I agree something needs to be fixed too. But I'd rather fix it in bin-wrapper if we can.

shinnn commented 9 years ago

I created another fork that use bin.path() but includes some performance improvements.

Here is time-require result:

Start time: (2015-01-27 00:27:54 UTC) [treshold=1%]
 #  module                          time  %
 1  text-table (no...ble/index.js)   1ms  ▇▇ 5%
 2  date-time (nod...ime/index.js)   2ms  ▇▇▇▇ 11%
 3  pretty-ms (nod...-ms/index.js)   2ms  ▇▇▇▇ 11%
 4  ansi-styles (n...si-styles.js)   2ms  ▇▇▇▇ 11%
 5  strip-ansi (no...nsi/index.js)   1ms  ▇▇ 5%
 6  has-color (nod...lor/index.js)   1ms  ▇▇ 5%
 7  chalk (node_mo...alk/index.js)   8ms  ▇▇▇▇▇▇▇▇▇▇▇▇▇ 42%
 8  bin-wrapper (n...per/index.js)   2ms  ▇▇▇▇ 11%
 9  ./lib (node_mo...lib/index.js)   4ms  ▇▇▇▇▇▇▇ 21%
10  cwebp-bin (nod...bin/index.js)   5ms  ▇▇▇▇▇▇▇▇ 26%
Total require(): 13
Total time: 19ms

Sufficiently fast. How about it?