Fix reduce memory usage #211
Conversation
Codecov Report
@@ Coverage Diff @@
## master #211 +/- ##
===========================================
- Coverage 98.2% 87.53% -10.68%
===========================================
Files 7 7
Lines 391 393 +2
Branches 157 152 -5
===========================================
- Hits 384 344 -40
- Misses 7 44 +37
- Partials 0 5 +5
Continue to review full report at Codecov.
|
| if (this.cache.isEnabled()) { | ||
| return this.cache.get(task).then((data) => data, enqueue); | ||
| } | ||
| return enqueue(task); |
There was a problem hiding this comment.
I don't think plimit makes any guarantee about the order of execution of the enqueued functions. I think this means that enqueue(task) can be called in any order and therefore task.callback(taskResult) can be, too.
This seems like it has the potential to introduce non-determinism whenever parallelism is enabled. That is, if I run my webpack build twice, I expect it to produce the same outputs. This change seems to break that guarantee whenever extractComments is on since comment extraction depends on processing the results in a particular order.
It is important that this plugin is deterministic because nondeterministic builds needlessly invalidate long-term caches.
There was a problem hiding this comment.
Yes, we need sort properties of allExtractedComments here https://github.com/webpack-contrib/terser-webpack-plugin/blob/master/src/index.js#L509
There was a problem hiding this comment.
I will fix it in near future
There was a problem hiding this comment.
Hm, not sure the problem exists:
We sort comments inside each file, so output is deterministic
Webpack sort assets by default
There was a problem hiding this comment.
Ok, I think you're right. Since comments are sorted and then de-duped after being collected in allExtractedComments, I think the output is deterministic.
This PR contains a:
Motivation / Use-Case
reduce memory usage
fixes #143
Breaking Changes
No
Additional Info