lukeapage / node-markdown-spellcheck

spellchecks markdown files
145 stars 58 forks source link

Speed regression #43

Closed kvz closed 9 years ago

kvz commented 9 years ago

Hey there,

Thanks for the fixes on this project :heart:

I'm writing to report a significant speed regression going from 0.4.0 to 0.6.0.

Where 0.4.0 completed scanning 196 (some fairly large) markdown files in 1.3 seconds:

kvz at mbp in ~/code/content on master*
$ npm install --save markdown-spellcheck@0.4.0 && time make test-spelling
markdown-spellcheck@0.4.0 node_modules/markdown-spellcheck
├── async@1.4.2
├── hunspell-spellchecker@1.0.2
├── marked@0.3.5
├── commander@2.8.1 (graceful-readlink@1.0.1)
├── chalk@1.1.1 (escape-string-regexp@1.0.3, supports-color@2.0.0, ansi-styles@2.1.0, has-ansi@2.0.0, strip-ansi@3.0.0)
├── glob@5.0.14 (path-is-absolute@1.0.0, inherits@2.0.1, inflight@1.0.4, once@1.3.2, minimatch@2.0.10)
└── inquirer@0.9.0 (figures@1.3.5, strip-ansi@3.0.0, ansi-regex@2.0.0, cli-width@1.0.1, through@2.3.8, readline2@0.1.1, run-async@0.1.0, rx-lite@2.5.2, lodash@3.10.1)
node_modules/.bin/mdspell \
        --report \
      --ignore-numbers \
      --ignore-acronyms \
      **/*.md _layouts/*.html _includes/*.html *.html **/robot_*.yml
    _posts/2015-09-14-nodevember.md
       45 | peakers-soledad-penades.jpg) [Soledad Penadés](http://twitter.com/s
       45 | soledad-penades.jpg) [Soledad Penadés](http://twitter.com/supersole
       47 | s](http://twitter.com/seldo), npm, Inc.
       52 | ://twitter.com/Aimee_Knight), Javascript Jabber               | *Guest
       58 | ter.com/HenrikJoreteg), Human Javascript           | *Guest Speaker*
       61 | ttp://twitter.com/tgriesser), knex / bookshelf
       63 | tp://twitter.com/ReBeccaOrg), npm, Inc.                     | I
       63 |                    | Into the npm installer – how it works and
       64 |     | These Bots are Made for Walkin'
       72 | ttp://twitter.com/aredridel), npm, Inc
       76 | ://twitter.com/dschenkelman), Auth0               | The dirty sec
       84 | ](http://twitter.com/VinoBS), Math for Game Developers        |
       84 | elopers        | Intuitive 3D Math Crash Course
       88 | peakers-kassandra-perch.jpg) [Kassandra Perch](http://twitter.com/nod
       90 | -speakers-parris-khachi.jpg) [Parris Khachi](http://twitter.com/pa
       92 | speakers-stacey-mulcahy.jpg) [Stacey Mulcahy](http://twitter.com/b
       93 | twitter.com/jacqueswoodcock), Tandum               | Username and
       93 | ords are dead. Mobile ALL the thingz.

>> 18 spelling errors found in 196 files
make[1]: *** [test-spelling-interactive] Error 1
make: *** [test-spelling] Error 2

real    0m1.341s
user    0m1.313s
sys 0m0.104s

0.6.0 is taking 1m46s:

kvz at mbp in ~/code/content on master*
$ npm install --save markdown-spellcheck@0.6.0 && time make test-spelling
markdown-spellcheck@0.6.0 node_modules/markdown-spellcheck
├── async@1.4.2
├── hunspell-spellchecker@1.0.2
├── marked@0.3.5
├── commander@2.8.1 (graceful-readlink@1.0.1)
├── chalk@1.1.1 (escape-string-regexp@1.0.3, supports-color@2.0.0, ansi-styles@2.1.0, strip-ansi@3.0.0, has-ansi@2.0.0)
├── glob@5.0.14 (path-is-absolute@1.0.0, inherits@2.0.1, inflight@1.0.4, once@1.3.2, minimatch@2.0.10)
└── inquirer@0.9.0 (figures@1.3.5, strip-ansi@3.0.0, ansi-regex@2.0.0, cli-width@1.0.1, through@2.3.8, run-async@0.1.0, rx-lite@2.5.2, readline2@0.1.1, lodash@3.10.1)
node_modules/.bin/mdspell \
        --report \
      --ignore-numbers \
      --ignore-acronyms \
      **/*.md _layouts/*.html _includes/*.html *.html **/robot_*.yml
    _posts/2015-09-14-nodevember.md
       45 | peakers-soledad-penades.jpg) [Soledad Penadés](http://twitter.com/s
       45 | soledad-penades.jpg) [Soledad Penadés](http://twitter.com/supersole
       47 | s](http://twitter.com/seldo), npm, Inc.
       52 | ://twitter.com/Aimee_Knight), Javascript Jabber               | *Guest
       58 | ter.com/HenrikJoreteg), Human Javascript           | *Guest Speaker*
       61 | ttp://twitter.com/tgriesser), knex / bookshelf
       63 | tp://twitter.com/ReBeccaOrg), npm, Inc.                     | I
       63 |                    | Into the npm installer – how it works and
       64 |     | These Bots are Made for Walkin'
       72 | ttp://twitter.com/aredridel), npm, Inc
       76 | ://twitter.com/dschenkelman), Auth0               | The dirty sec
       84 | ](http://twitter.com/VinoBS), Math for Game Developers        |
       84 | elopers        | Intuitive 3D Math Crash Course
       88 | peakers-kassandra-perch.jpg) [Kassandra Perch](http://twitter.com/nod
       90 | -speakers-parris-khachi.jpg) [Parris Khachi](http://twitter.com/pa
       92 | speakers-stacey-mulcahy.jpg) [Stacey Mulcahy](http://twitter.com/b
       93 | twitter.com/jacqueswoodcock), Tandum               | Username and
       93 | ords are dead. Mobile ALL the thingz.

>> 196 files are free from spelling errors
make[1]: *** [test-spelling-interactive] Error 1
make: *** [test-spelling] Error 2

real    1m46.542s
user    1m43.949s
sys 0m2.971s

I do very much like the added --en-us feature in 0.6.0 so I hope we can figure this out.

lukeapage commented 9 years ago

I think I fixed it, I guessed the only thing that could have made it that much slower.

There was some refactoring in 0.5.0 as well as the bugfix and I think that introduced it.

It is released as 0.6.1

kvz commented 9 years ago

Confirmed!