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

Improve build time #6309

Closed
anandthakker opened this issue Mar 9, 2018 · 8 comments
Closed

Improve build time #6309

anandthakker opened this issue Mar 9, 2018 · 8 comments
Assignees

Comments

@anandthakker
Copy link
Contributor

Follow-up to #6196

On my laptop, rebuilding takes about 7s:

  • 3.2s to build rollup/build/{index,worker,chunk1}.js, and then
  • 4s just to assemble these into the final bundle

That second step is much slower than it needs to be. It's using Rollup again, but only to make sure the sourcemaps for {index,worker,chunk1}.js get properly consumed/composed into the final sourcemap. This means we're wasting lots of time re-parsing these ast's even though it's not at all necessary.

@mourner
Copy link
Member

mourner commented Mar 9, 2018

Here's a perf profile for a build from scratch: https://gist.github.com/mourner/0acdc68648627ac431229d2ad601eefc

A quick scan seems to suggest there are some nice low hanging fruits there.

@anandthakker
Copy link
Contributor Author

See also rollup/rollup#1553

@anandthakker
Copy link
Contributor Author

Tracking with a project board here: https://github.com/mapbox/mapbox-gl-js/projects/11

@jfirebaugh
Copy link
Contributor

Rollup 0.58.0 and later includes rollup/rollup#2119. We're currently on 0.57.1; however upgrading to 0.58.2 seems to break our solution for Web Workers:

$ /Users/john/Development/mapbox-gl-js/node_modules/.bin/rollup -c --environment BUILD:production

src/index.js, src/source/worker.js → rollup/build/mapboxgl...
(!) Circular dependency: src/style-spec/expression/parsing_context.js -> src/style-spec/expression/compound_expression.js -> src/style-spec/expression/parsing_context.js
(!) Circular dependency: src/style-spec/validate/validate.js -> src/style-spec/validate/validate_function.js -> src/style-spec/validate/validate.js
(!) Circular dependency: src/style-spec/validate/validate.js -> src/style-spec/validate/validate_function.js -> src/style-spec/validate/validate_object.js -> src/style-spec/validate/validate.js
(!) Circular dependency: src/style-spec/validate/validate.js -> src/style-spec/validate/validate_function.js -> src/style-spec/validate/validate_array.js -> src/style-spec/validate/validate.js
(!) Circular dependency: src/style-spec/validate/validate.js -> src/style-spec/validate/validate_layer.js -> src/style-spec/validate/validate_paint_property.js -> src/style-spec/validate/validate_property.js -> src/style-spec/validate/validate.js
(!) Circular dependency: src/style-spec/validate/validate.js -> src/style-spec/validate/validate_layer.js -> src/style-spec/validate/validate.js
(!) Circular dependency: src/style-spec/validate/validate.js -> src/style-spec/validate/validate_light.js -> src/style-spec/validate/validate.js
(!) Circular dependency: src/geo/lng_lat_bounds.js -> src/geo/lng_lat.js -> src/geo/lng_lat_bounds.js
(!) Circular dependency: src/index.js -> src/ui/map.js -> src/style/style.js -> src/util/global_worker_pool.js -> src/util/worker_pool.js -> src/util/browser/web_worker.js -> src/index.js
(!) Circular dependency: src/index.js -> src/ui/map.js -> src/style/style.js -> src/util/global_worker_pool.js -> src/util/worker_pool.js -> src/index.js
created rollup/build/mapboxgl in 15.7s

rollup/mapboxgl.js → dist/mapbox-gl.js...
[!] Error: Could not resolve './build/mapboxgl/chunk1' from rollup/mapboxgl.js
Error: Could not resolve './build/mapboxgl/chunk1' from rollup/mapboxgl.js
    at error (/Users/john/Development/mapbox-gl-js/node_modules/rollup/dist/rollup.js:199:15)
    at /Users/john/Development/mapbox-gl-js/node_modules/rollup/dist/rollup.js:20674:25
    at <anonymous>

@mourner
Copy link
Member

mourner commented Apr 30, 2018

@guybedford does this ring a bell? Probably related to your recent chunk-related changes...

@guybedford
Copy link

@mourner no idea what that might be actually. It seems like the resolveId plugin function is not returning anything for that case. Let me know if you can isolate this a little more perhaps.

@anandthakker
Copy link
Contributor Author

anandthakker commented Apr 30, 2018 via email

@guybedford
Copy link

Note a quick fix here would be to add chunkNames: '[alias].js' to the Rollup configuration, to avoid adding the hash.

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

4 participants