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

Bundle into a flat file #4230

Closed
wants to merge 1 commit into from
Closed

Conversation

sophiebits
Copy link
Collaborator

Just sharing. Can look at incorporating this into our build pipeline if we want to. Produces files that look like https://gist.github.com/spicyj/19df83d04eaad215b92b.

$ node pack-flat.js >react-flat.js
$ cat react-flat.js | ./node_modules/.bin/uglifyjs -c -m >react-flat.min.js

Curiously, it's bigger after gzipping than the other way:

$ wc -c react-flat.js react-flat.min.js build/react.js build/react.min.js
  662769 react-flat.js
  121416 react-flat.min.js
  601727 build/react.js
  124097 build/react.min.js
 1510009 total
$ gzip <react-flat.min.js | wc -c
   38355
$ gzip <build/react.min.js | wc -c
   36602

Initial startup execution cost looks to be about 14% less though.

@sophiebits
Copy link
Collaborator Author

cc @sebmarkbage

cc @sebmck – In case you're interested. I have a transformer here to prefix all top-level variables and extract/rewrite requires. Maybe you have a better suggestion for how to provide arguments to a transformer (currentModule) and how to get some sort of result out (required)?

@sebmarkbage
Copy link
Collaborator

Maybe it is because the minifier has to use longer identifiers to make them globally unique?

Just sharing. Can look at incorporating this into our build pipeline if we want to. Produces files that look like https://gist.github.com/spicyj/19df83d04eaad215b92b.

```
$ node pack-flat.js >react-flat2.js
$ cat react-flat.js | ./node_modules/.bin/uglifyjs -c -m >react-flat2.min.js
```

Curiously, it's bigger after gzipping than the other way:

```
$ wc -c react-flat2.js react-flat2.min.js build/react.js build/react.min.js
  615136 react-flat2.js
  118944 react-flat2.min.js
  601727 build/react.js
  124097 build/react.min.js
$ gzip <react-flat2.min.js | wc -c
   36090
$ gzip <build/react.min.js | wc -c
   36602
```

Initial startup cost looks to be about 22% less though.
@sophiebits
Copy link
Collaborator Author

Updated it to eliminate variables from requires completely, so

// foo.js
var invariant = require('invariant');
invariant(bar);

becomes

invariant(foo$$bar);

instead of

var foo$$invariant = invariant;
foo$$invariant(foo$$bar);

Now gzip is better:

$ gzip <react-flat2.min.js | wc -c
   36090
$ gzip <build/react.min.js | wc -c
   36602

I also remeasured the perf and it takes now 22% less time overall to execute the minified bundle (previously 14%)! Guess those variables aren't free.

@sebmarkbage
Copy link
Collaborator

Well this is a giant function so I would expect V8 to completely ignore optimizing it which is fine since it is executed once. It's probably just interpreted as part of interpretation they probably keep the variables around in case they get accessed later, since they probably don't know if they will be.

Uglify should've stripped them though. Have your tried Closure's simple mode?

@sophiebits
Copy link
Collaborator Author

My last test was actually jsc but just tested in node and got 33% improvement (30% before my last change).

Uglify doesn't, unless I'm just missing a switch:

$ echo '(function() { var foo = 2; var bar = foo; alert(bar); })()' | ./node_modules/.bin/uglifyjs -m -c
!function(){var a=2,n=a;alert(n)}();

but Closure simple does give

(function(){alert(2)})();

on the same input. Let me see if that changes things…

@sophiebits
Copy link
Collaborator Author

Running through Closure simple mode (where I manually readded a "use strict" since it doesn't want to keep it) brings us to 31% better in jsc, 40% better in node.

$ gzip <react-flat2.cc.js | wc -c
   32299

@sebmarkbage
Copy link
Collaborator

Neat. The real power comes from the more powerful dead code elimination, we've yet to taken advantage of, and advanced mode.

Now all we have to figure out is where we'll draw the line of which modules to include (e.g. fbjs etc.) and how we're going to use this internally were we override some modules.

@bloodyowl
Copy link
Contributor

have you tried to bundle react with webpack? usually bundles are small and it wouldn't make you depend on a local babel plugin

@sebmck
Copy link
Contributor

sebmck commented Jun 26, 2015

The file would be a lot bigger.

@sophiebits
Copy link
Collaborator Author

@bloodyowl A long, long time ago: #1937. We made other improvements to the browserify build since though to make them more comparable. This change here is still significant.

@sophiebits
Copy link
Collaborator Author

cc @amasad -- you may be interested in the gains we saw here. To summarize, we ended up with 31% better in jsc, 40% better in node for initial startup and 12% smaller bundle after min+gzip. I imagine the runtime gains might be higher for a bundle with more modules than React has.

@amasad
Copy link
Contributor

amasad commented Aug 18, 2015

@spicyj this is awesome, thanks for sharing.

@gaearon
Copy link
Collaborator

gaearon commented Mar 26, 2016

Closing in favor of #6351. Let’s discuss this further in there!

@gaearon gaearon closed this Mar 26, 2016
@keyz keyz mentioned this pull request Jul 3, 2016
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants