firstandthird / load-grunt-config

Grunt plugin that lets you break up your Gruntfile config by task
firstandthird.github.io/load-grunt-config/
MIT License
374 stars 64 forks source link

Low hanging optimizations #95

Closed webuniverseio closed 9 years ago

webuniverseio commented 10 years ago

Hi, I've made couple of simple optimizations which increase module loading speed. I've run tests and everything is passing just fine. Please review and let me know what you think. Thanks.

jgallen23 commented 10 years ago

Why is lodash faster than lodash-node?

webuniverseio commented 10 years ago

Because lodash-node stores modules separately and it you import the whole library, then it have to import each module (that would be a lot of file read operations). Importing individual modules might be even faster, I'll check that in the evening and let you know.

jgallen23 commented 10 years ago

Makes sense. We should just load the modules we need.

For yaml, what makes putting the require in the if statement faster?

webuniverseio commented 10 years ago

Regarding loading modules separately, what if at certain point you'll want to use more of lodash functionality - then you'll have to include more modules and they in turn might require more dependencies - in the end it will increase number of disk reads. Regular lodash will import concatenated file only once, meaning that it will be quickly imported while giving you plenty of functionality.

As for yaml it makes it faster if user don't have yaml files to parse - which is my case. Otherwise it will take the same amount of time. Grunt itself have the same issue - it requires yaml module, which takes time to read from disk and parse, and that slows things down for people who don't use that format.

webuniverseio commented 10 years ago

It could be that you might be confused about why I even care about those cases, but the issue comes from multiple plugins and I basically need plugin authors to help with optimizations. More background about the issue which I'm trying to solve with those PRs could be found here -> https://github.com/shama/grunt-gulp/commit/5b4c5050a33a0dfe14fa0a8b32d69ea3fbc5eece#commitcomment-8531006.

Please let me know what you think. Thank you.

mattkime commented 10 years ago

What use case does this improve? What code are you using to test?

webuniverseio commented 10 years ago

Hi @mattkime, if lodash-node will be replaced with lodash it will speed up start up time for everyone (for reasons described above). If developer doesn't use yaml files he will get additional improvements in start up time. For that particular plugin we're talking about a few hundreds of milliseconds which sounds like a very small number. But in combination with other plugins that could be seconds. Depending on dev's machine CPU power it could be very slow. That is not an issue of a particular plugin, it is an issue of a system working as a whole. Hope that makes sense.

mattkime commented 10 years ago

nice work, definitely important, particularly for larger projects.

mattkime commented 9 years ago

@jgallen23 i'm curious if there's anything i can do to move this along @szarouski perhaps it would be helpful if you could display the improvement in total load time saved.

on my box, using load-contrib-clean appears to increase my "loading tasks" time from 80ms to 180ms. thats using time-grunt.

jgallen23 commented 9 years ago

Can you merge master into your pull request and I'll get it in the next release

webuniverseio commented 9 years ago

@jgallen23, sure I'll do it soon.

jgallen23 commented 9 years ago

thanks

webuniverseio commented 9 years ago

Question, it was quite easy to merge, but when I run npm test I got bunch of errors. I then cloned original firstandthird/load-grunt-config, to verify if there are existing issues with tests, and tried npm test there, however I'm still getting bunch of errors both on windows and debian. See screen-shot below, is it normal?

2015-04-12_144009

After merge I'm getting same number of failing tests, so I dare to assume that my changes are not a reason of these failures.

jgallen23 commented 9 years ago

I'll look into the failing tests. I merged a bunch of other stuff in today.

webuniverseio commented 9 years ago

Merge is done.