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

add output.futureEmitAssets #8642

Merged
merged 2 commits into from Jan 19, 2019

Conversation

Projects
None yet
4 participants
@sokra
Copy link
Member

sokra commented Jan 17, 2019

add a new version of emitting assets which allows to free memory of Sources with the trade-off of disallowing reading asset content after emitting

It also uses Source.buffer when available.

What kind of change does this PR introduce?
memory

Did you add tests for your changes?
no (it will be tested in the next branch, when it's on by default)

Does this PR introduce a breaking change?
no

What needs to be documented once your changes are merged?
New option output.futureEmitAssets allows to opt-in to the new way of emitting assets which allows to free memory of Sources with the trade-off of disallowing reading asset content after emitting

add output.futureEmitAssets
add a new version of emitting assets which allows to free memory of Sources with the trade-off of disallowing reading asset content after emitting

It also uses Source.buffer when available.
@webpack-bot

This comment has been minimized.

Copy link

webpack-bot commented Jan 17, 2019

For maintainers only:

  • This need to be documented (issue in webpack/webpack.js.org will be filed when merged)
@webpack-bot

This comment has been minimized.

Copy link

webpack-bot commented Jan 17, 2019

Thank you for your pull request! The most important CI builds succeeded, we’ll review the pull request soon.

@dav-is

dav-is approved these changes Jan 17, 2019

Copy link
Contributor

dav-is left a comment

Fixes the memory issue on our end

@dav-is

This comment has been minimized.

Copy link
Contributor

dav-is commented Jan 17, 2019

When using the flag on a development build, it freezes on WebpackDevMiddleware. It doesn't appear to throw any errors. It accesses source() after emit https://github.com/webpack/webpack-dev-middleware/blob/90d0d94c76e51c3b55cdd2b19978c8cd78edcf85/lib/fs.js#L28

@sokra

This comment has been minimized.

Copy link
Member Author

sokra commented Jan 19, 2019

You don't have to use writeToDisk

@sokra sokra merged commit 6389e41 into master Jan 19, 2019

11 of 13 checks passed

codecov/patch/integration 26.22% of diff hit (target 90%)
Details
codecov/project/integration 91.17% (-0.23%) compared to 03ffa48
Details
codecov/changes/basic No unexpected coverage changes found.
Details
codecov/changes/integration No unexpected coverage changes found.
Details
codecov/changes/unit No unexpected coverage changes found.
Details
codecov/patch/basic Coverage not affected when comparing 03ffa48...6e383cf
Details
codecov/patch/unit Coverage not affected when comparing 03ffa48...6e383cf
Details
codecov/project/basic 100% remains the same compared to 03ffa48
Details
codecov/project/unit 100% (target 0%)
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
licence/cla Contributor License Agreement is signed.
Details
security/snyk - package.json (sokra) No new issues
Details

@sokra sokra deleted the memory/future-emit-assets branch Jan 19, 2019

filipesilva added a commit to filipesilva/angular-cli that referenced this pull request Jan 22, 2019

filipesilva added a commit to filipesilva/angular-cli that referenced this pull request Jan 22, 2019

mgechev added a commit to angular/angular-cli that referenced this pull request Jan 22, 2019

@manniL manniL referenced this pull request Feb 11, 2019

Merged

perf(webpack): use `futureEmitAssets` #5003

2 of 5 tasks complete

Kiku-git added a commit to Kiku-git/angular-cli that referenced this pull request Mar 7, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.