Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Require takes a lot of time #76

Closed
sbmaxx opened this issue Aug 11, 2015 · 8 comments
Closed

Require takes a lot of time #76

sbmaxx opened this issue Aug 11, 2015 · 8 comments
Assignees
Milestone

Comments

@sbmaxx
Copy link

sbmaxx commented Aug 11, 2015

Here's small test script:

var p = techs[tech];
console.time('require(' + tech + ') = ' + p);
techs[tech] = require(techs[tech]);
console.timeEnd('require(' + tech + ') = ' + p);

And the results:

require(files) = enb-bem-techs/techs/files: 28ms
require(levels) = enb-bem-techs/techs/levels: 14ms
require(autoprefixer) = enb-autoprefixer: 378ms
require(stylus) = enb-stylus/techs/css-stylus: 92ms
require(provider) = enb/techs/file-provider: 3ms
require(deps) = enb-bem-techs/techs/deps-old: 9ms
require(priv) = enb-includes/techs/priv: 4ms
require(decl) = enb-bem-techs/techs/deps-by-tech-to-bemdecl: 3ms
require(i18n-merge-keysets) = enb-bem-i18n/techs/i18n-merge-keysets: 16ms
require(i18n-lang-js) = enb-bem-i18n/techs/i18n-lang-js: 15ms
require(pub) = enb-includes/techs/pub: 1ms
require(very-strange-deps) = ./techs/deps-file-provider: 2ms
require(deps-subtract) = enb-bem-techs/techs/subtract-deps: 2ms
require(bemhtml) = enb-xjst/techs/bemhtml: 344ms
require(i18n-lang-js-dev) = ./techs/i18n-lang-js: 1ms
require(borschik) = enb-borschik/techs/borschik: 74ms

Requiring of bemhtml-xjst takes a lot of time each build. Maybe it's time to refactor it? :) The common pattern for solving issues like this — require dependencies only on build action

@sbmaxx
Copy link
Author

sbmaxx commented Aug 11, 2015

For example, enb-autoprefixer with a very long init time too:

before:

require(autoprefixer) = enb-autoprefixer: 378ms

before:

var autoprefixer = require('autoprefixer-core');
var browserslist = require('browserslist');

module.exports = require('enb/lib/build-flow').create()
    .name('css-autoprefixer')
    .defineRequiredOption('sourceTarget')
    .optionAlias('sourceTarget', 'source')
    .defineOption('destTarget')
    .optionAlias('destTarget', 'target')
    .defineOption('browserSupport')
    .target('destTarget', '?.css')
    .useSourceText('sourceTarget')
    .builder(function (css) {
        var prefixer = autoprefixer({
            browsers: this._browserSupport || browserslist.defaults
        });

        try {
            return prefixer.process(css).css;
        } catch (e) {
            throw new Error(e);
        }
    })
    .createTech();

after:

require(autoprefixer) = enb-autoprefixer: 8ms

The changes.

after:

module.exports = require('enb/lib/build-flow').create()
    .name('css-autoprefixer')
    .defineRequiredOption('sourceTarget')
    .optionAlias('sourceTarget', 'source')
    .defineOption('destTarget')
    .optionAlias('destTarget', 'target')
    .defineOption('browserSupport')
    .target('destTarget', '?.css')
    .useSourceText('sourceTarget')
    .builder(function (css) {

        var autoprefixer = require('autoprefixer-core');
        var browserslist = require('browserslist');

        var prefixer = autoprefixer({
            browsers: this._browserSupport || browserslist.defaults
        });

        try {
            return prefixer.process(css).css;
        } catch (e) {
            throw new Error(e);
        }
    })
    .createTech();

Now it will only takes a lot of time when we need to build/rebuild autoprefixer's target.

This was referenced Aug 11, 2015
@blond blond added the review label Aug 11, 2015
@arikon
Copy link
Member

arikon commented Aug 11, 2015

@sbmaxx So what will be the profit? In you pr require() will be called on each target. This require() call is not free, and will postpone the build result.

@sbmaxx
Copy link
Author

sbmaxx commented Aug 11, 2015

Faster build if we didn't modify bemhtml files. 80% percentage of builds in
our projects ;)

If I need just to rebuild css file I don't want to wait 300ms for bemhtml
require

вторник, 11 августа 2015 г. пользователь Sergey Belov написал:

@sbmaxx https://github.com/sbmaxx So what will be the profit?


Reply to this email directly or view it on GitHub
#76 (comment).

wbr,
Roman Rozhdestvenskiy

@arikon
Copy link
Member

arikon commented Aug 11, 2015

@sbmaxx Are you using enb server? If not — why?

So this speed up on enb make invokation will slow overall enb make performance in continuous integration. Is it a good exchange in performance? What do you think?

@sbmaxx
Copy link
Author

sbmaxx commented Aug 12, 2015

So this speed up on enb make invokation will slow overall enb make performance in continuous integration

Why it be slow? Node.js has a good require cache ;) Once module was required in build action it will be cached.

Are you using enb server? If not — why?

Not in all project :/ Kind of legacy and not frendly infrastructure to run another http-server (one for each dev), change ports, links, etc.

@blond
Copy link
Member

blond commented Aug 22, 2015

So this speed up on enb make invokation will slow overall enb make performance in continuous integration. Is it a good exchange in performance? What do you think?

@arikon, I think that the performance does not suffer.

If you write a simple benchmark:

require('enb-xjst/techs/bemhtml');

console.time('bemhtml-100');
for (var i = 0; i < 100; ++i) {
    require('enb-xjst/techs/bemhtml');
}
console.timeEnd('bemhtml-100');

You can see that speed of building project will suffer for 1ms.

bemhtml-100: 1ms

In [email protected] we do the same: https://github.com/enb-bem/enb-xjst/blob/master/techs/xjst.js#L71.

Before lazy load:

bemhtml-tech: 623ms
bemhtml-100: 1ms

After :

bemhtml-tech: 191ms
bemhtml-100: 1ms

But still long because require browserify: https://github.com/enb-bem/enb-xjst/blob/master/lib/bundle.js#L4.

If you move require of browserify, then:

bemhtml-tech: 8ms
bemhtml-100: 1ms

@blond
Copy link
Member

blond commented Aug 22, 2015

@sbmaxx I am not against such changes, but I do not like is that it will have to do in each package.

Is it critical?

Can we solve the problem differently? For example, think about how not to run an assembly via the CLI.

@sbmaxx
Copy link
Author

sbmaxx commented Aug 28, 2015

@blond we're thinking about it, but in 90% of cases we use CLI :/

I did benchmarking for other techs that we're using. The most slow are enb-xjst and enb-autoprefixer. All others are fine.

@blond blond added this to the 2.0 milestone Aug 31, 2015
@blond blond self-assigned this Aug 31, 2015
@blond blond closed this as completed in #93 Aug 31, 2015
@blond blond removed the review label Aug 31, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants