Backport #308 to apply to 0.6
benthemonkey opened this issue · 10 comments
Many libraries won't be upgrading to source-map 0.7 until Node 6 is out of the LTS schedule (example: webpack/webpack-sources#34 (comment)). That is still 5-6 months away.
When I upgraded my project from Node 8 to Node 10, I was surprised to see that the performance of source-map generation was significantly diminished. A webpack production build went from taking 4 minutes to 20 minutes, and all that extra time can be attributed to source-map generation. Thanks to this issue thread -- webpack-contrib/uglifyjs-webpack-plugin#272 (comment) -- it looks like #308 fixes the issue. I'm hoping it's possible to take @marcins 's suggestion, backport this significant performance improvement, and release it as 0.6.2.
I've made the necessary changes to backport the performance improvements in #308 on my fork: 0.6.1...benthemonkey:patch-0.6.1
With these changes I found that my production build time goes back to taking 4 minutes.
@loganfsmyth or @fitzgen I'm sorry if this is an improper way to try to get your attention, but I'd love it if you could consider my proposal and state whether it is something you'd be willing to consider.
I'm sure this change will help many other developers using source-maps 0.6.x with Node 10, but I'd understand if you aren't interested in addressing new issues that may result from this change.
I'd very much love to see this fix back ported as well, it's causing us a lot of headaches.
I would love to see this PR.
In my opinion, taking a v0.6 library of this scope and releasing a minor version update that changes the API from sync to async, introduces a new dependency on WASM, with no plans to support the older API going forward or advice on how to handle (then current) versions of nodejs, was totally ill-advised.
There's already been a request to create a support branch for 0.6.x (#324), which was closed; without a support branch to merge into, I don't see how to even get the conversation started about additional changes to 0.6.x.
Another option is potentially a new synchronous API, like #369; doesn't solve the pre-WASM node problem, but those are rolling off pretty quickly now (even for companies using them past end of life).
FWIW I think it was a huge mistake to have ever introduced this WASM build into source-map; it should have been a separate branch or even a separate project.
I've started looking at our Webpack builds again, and have had some pretty good results with using patch-package to apply @benthemonkey's patch (thanks Ben!) to source-maps v0.6.x in our node_modules during post-install.
Initial tests show the patched source-maps takes our full Webpack 3 production build times (with source maps, of course) from ~35 minutes in CI to ~12 minutes. (This is some 1.3 million lines of source, so we'd be pretty darn happy with 12 min)
Memory / perf issues were blocking us from upgrading to Webpack 4 previously, but I'm hoping with this approach we can do that too, that's my task for the rest of the week 🤞
I also applied the patch and saw build time with Terser & Source-maps drop from over 6 minutes, to just 1.5 minutes. PLEASE PLEASE PLEASE consider back-porting this to 0.6.x.
Many libraries won't be upgrading to source-map 0.7 until Node 6 is out of the LTS schedule (example: webpack/webpack-sources#34 (comment)). That is still 5-6 months away.
Has this changed now that node 6 is out of LTS?
@jkrems it didn't. You can check the versions tab for stats https://www.npmjs.com/package/source-map
Most of the libraries stuck on 0.6.1 and 0.5.7. Maybe you could consider to backport changes from https://github.com/7rulnik/source-map-js and release it as 0.6.2? With these two PRs 7rulnik#3 7rulnik#2 it's just 2x slower than the current WASM implementation but still has sync JS only API.
It has now an even bigger difference. @jkrems would you consider merging a PR if one would be created?
I think that would be a reasonable short-term fix. Though I'll have to check the release process to make sure we can also publish it to npm.