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

Speed up min builds #1933

Closed
zpao opened this issue Jul 25, 2014 · 8 comments
Closed

Speed up min builds #1933

zpao opened this issue Jul 25, 2014 · 8 comments

Comments

@zpao
Copy link
Member

zpao commented Jul 25, 2014

We're minifiying twice thanks to uglifyify. We need this to remove dead requires. But it also doesn't minify all the way, so we still need uglify as well. I haven't actually looked to see if there's a better way.

@sophiebits
Copy link
Collaborator

I believe webpack is smart enough to strip out unneeded requires even before uglification so that might be faster than what we're doing…

@syranide
Copy link
Contributor

@spicyj Is it? I haven't actually checked, but I use require('es5-shim') to apply the shim and it's working in IE8 so I really doubt it's been stripped (unless it's really smart and knows when a module has side-effects).

@sophiebits
Copy link
Collaborator

I meant if you do if (false) { require('dev-only-stuff'); } then webpack won't include that module.

@zertosh
Copy link
Member

zertosh commented Jul 27, 2014

@zpao, I had a similar problem with "dead requires" so I put together unreachable-branch - it's a browserify transform (jstransform visitor under-the-hood) to comment out unreachable branches in if's, ? ternaries and || && logical operations.

@zpao
Copy link
Member Author

zpao commented Oct 16, 2014

Some context... I did end up trying unreachable-branch and it mostly works (except it chokes on if (__DEV__) { /* comment */ code }, which we have at least 1 instance of). Even fixing that I didn't end up with any significant perf wins (and I even combined the multiple jstransforms we have happening at the browserify step).

@tgriesser
Copy link

@spicyj is right that webpack is not only smart enough to strip out dead requires, but it strips out dead code paths entirely, so in your build (pre-minified code) instead of:

("production" !== "development" ? invariant(
    children.nodeType !== 1,
    'traverseAllChildren(...): Encountered an invalid child; DOM ' +
    'elements are not valid children of React components.'
) : invariant(children.nodeType !== 1));

you just get the much nicer:

invariant(children.nodeType !== 1,
  'traverseAllChildren(...): Encountered an invalid child; DOM ' +
  'elements are not valid children of React components.'
);

@zertosh
Copy link
Member

zertosh commented Jan 24, 2015

wow I totally missed your response @zpao. I just pushed an updated version of unreachable-branch-transform that uses recast instead of jstransform, that doesn't have that problem. I did a quick time benchmark between this vs an extra uglify pass, that the time difference was negligible.

Though, I was able to shave off 2 sec by turning off mangle and a few compress options on the first pass of uglifyify - since it's only the dead code removal that matters. For the 2 sec improvement (prob less on faster machines), it wasn't worth PRing the changes to the grunt task that allow passing options to transforms 😞

The real killer is derequire.

@zpao
Copy link
Member Author

zpao commented Jul 28, 2015

I think at this point things are pretty fast. We'll make some more changes to the build process in general.

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

5 participants