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

after updating from 3.12.0 to 3.13.1 build time increase from 1m 14s to over 60m #204

Closed
StefanSchoof opened this issue Dec 22, 2018 · 50 comments

Comments

@StefanSchoof
Copy link

Bug report or Feature request?
bug

Version (complete output of terser -V)
3.13.1
Complete CLI command or minify() options used
I use terser over webpack. My code is found at https://github.com/StefanSchoof/espresso/tree/terser/func
The code is small (two typescript files; 59 lines and 95 lines for test)
The Build at 3.12.0 works in 1m 14 https://dev.azure.com/sschoof/espresso/_build/results?buildId=341 (click at function job to see log)
The Build at 3.13.0 fails after 54s with a cannot read property start of undefined: https://dev.azure.com/sschoof/espresso/_build/results?buildId=342 (click at function job to see log)
The Build at 3.13.1 got cancelled after 60 minutes. https://dev.azure.com/sschoof/espresso/_build/results?buildId=343 (there are other failing builds with 3.13.1: https://dev.azure.com/sschoof/espresso/_build/results?buildId=331, https://dev.azure.com/sschoof/espresso/_build/results?buildId=326)
The only difference between these are the terser version. (See https://github.com/StefanSchoof/espresso/commits/terser)

terser input

terser output or error

Expected result

@fabiosantoscode
Copy link
Collaborator

Did you re-run the build to make sure? Happy hols :)

@StefanSchoof
Copy link
Author

@fabiosantoscode
Copy link
Collaborator

I'm going to investigate, but can you possibly remove code until we find the offender?

@StefanSchoof
Copy link
Author

I am currently have only a smartphone with an ssh connection to a Linux with the code. This reduces my ways to inspect the issue. When I am back at my PC I will give it a try

@fabiosantoscode
Copy link
Collaborator

Coolsies :) I'll keep investigating, this bug is terrible.

@fabiosantoscode
Copy link
Collaborator

Can't reproduce on my machine :( will try in a VM

@StefanSchoof
Copy link
Author

On a Standard DS2 v2 (2 vcpus, 7 GB memory) Azure VM I can reproduce. I reduced the causing code in https://github.com/StefanSchoof/espresso/blob/terser/func/src/switch/switch.ts a little bit.

@fabiosantoscode
Copy link
Collaborator

fabiosantoscode commented Dec 26, 2018

Ok, thanks for reducing the code. I was having trouble with typescript but used your own now. Totally reproduced.

@fabiosantoscode
Copy link
Collaborator

Do you also compile the test?

@fabiosantoscode
Copy link
Collaborator

Can you try to make this only go through terser? For example, compile the TS to another file and run terser on that file to see if it still freezes.

I was wrong about reproducing it before, I was just hitting #202 and thought I found the issue.

@StefanSchoof
Copy link
Author

When I run

npx webpack

it takes forever.

When I run

npx webpack --mode development
npx terser dist/switch/switch.js

I get a result fast.

@StefanSchoof
Copy link
Author

I removed the test file and the problem still occurs

@StefanSchoof
Copy link
Author

I replaced the typescript with the js from the typescript compiler. Still occurs
https://github.com/StefanSchoof/espresso/blob/terser/func/src/switch/switch.js

@fabiosantoscode
Copy link
Collaborator

What's the command you're using @StefanSchoof ? I'm creating a new VM to try and reproduce. Make sure you pass '-mc' after the filename!

@StefanSchoof
Copy link
Author

I just start webpack (npx webpack) and have no configuration for terser (I was not aware that webpack used terser until I hit this bug). I think I have to look at the webpack terser plugin, to see how they call terser.

@fabiosantoscode
Copy link
Collaborator

I've created webpack-contrib/terser-webpack-plugin#48. Can you fill in your details?

@StefanSchoof
Copy link
Author

I did it. Thanks for your help, especially since the issue seams not be in terser.

@StefanSchoof
Copy link
Author

I was able to create a pure terser repro (without webpack):
I added a terser.js which tries to minify the input.js. Running a

node terser.js

seems to take forever.

@nifgraup
Copy link
Contributor

Regression point 842392e, bisected with terser.js & input.js as test case.

@fabiosantoscode
Copy link
Collaborator

Sorting through this mess. I'm still at 23325 lines. Next time I'm making you guys make the file smaller :P

@StefanSchoof
Copy link
Author

I heard there is a tool named terser to reduce the file size ;)

@fabiosantoscode
Copy link
Collaborator

Hahahaha :)

@fabiosantoscode
Copy link
Collaborator

Binary search is a blessing though, this could take days otherwise.

@StefanSchoof
Copy link
Author

Had you tried add some logging into 842392e#diff-00fe37b20ddfb9ecc9c492b0725fa2b2R1232 to see which code part loops there at the end?

With no knowledge of the code, I think it tries to optimize the then part and then an other thing reverts this and so on.

@fabiosantoscode
Copy link
Collaborator

I've tried that, that's why I'm reducing the bundle. 105 lines...

@fabiosantoscode
Copy link
Collaborator

I reduced too much and it turned out I was just not waiting enough time for it to build, but it looks like this is happening when minifying the xpath library you're using. Where can I find the source code?

@StefanSchoof
Copy link
Author

I think this is an sub dependency of ms-rest-azure and is located at: https://github.com/yaronn/xpath.js

@fabiosantoscode
Copy link
Collaborator

Temporarily reverted 842392e, will release a beta version so you can test it.

@fabiosantoscode
Copy link
Collaborator

Published 3.14.0-beta, can you try it @StefanSchoof ?

@Conduitry
Copy link
Contributor

This beta doesn't seem to include the necessary dist/bundle.js. Can you mark the old version as latest again in npm? I was just bitten by this when updating dependencies.

@StefanSchoof
Copy link
Author

My tests are somewhat inconclusive. Running the terser.js against the input.js succeed with the 3.14.0-beta, but my webpack build still does not succeed: https://dev.azure.com/sschoof/espresso/_build/results?buildId=372

Try to investigate further...

@StefanSchoof
Copy link
Author

Okay, found the cause for this behavior. My project had an dependency to the beta version, but terser-webpack-plugin stayed at the 1.13.1 version. After I forced to use this it webpack works again in under one minute.

@fabiosantoscode
Copy link
Collaborator

Thanks for informing me @Conduitry I wasn't aware that beta versions became latest on release.

@fabiosantoscode
Copy link
Collaborator

It's settled then. -b dies and the commit stays reverted. This will take some work, so if someone would volunteer that would be great

@StefanSchoof
Copy link
Author

@fabiosantoscode you can set an other tag then latest for an beta release, see https://docs.npmjs.com/cli/publish

@fabiosantoscode
Copy link
Collaborator

fabiosantoscode commented Dec 31, 2018

I already did! But next time I'll use that

@eltonio450
Copy link

Hello,

It seems we are facing an issue that looks like this one on CircleCI. I am not sure however which version is affected by the bug and which is not. And it seems related but not 100% sure...

The issue is either (1) a timeout (30 minutes, which occurs only in CircleCI) or (2) an error such as:

Error: EFAULT: bad address in system call argument, read
    at Object.readSync (fs.js:494:3)
    at tryReadSync (fs.js:333:20)
    at Object.readFileSync (fs.js:362:19)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:699:20)
    at Module.load (internal/modules/cjs/loader.js:599:32)
    at tryModuleLoad (internal/modules/cjs/loader.js:538:12)
    at Function.Module._load (internal/modules/cjs/loader.js:530:3)
    at Module.require (internal/modules/cjs/loader.js:637:17)
    at require (internal/modules/cjs/helpers.js:22:18)
    at Object.<anonymous> (/home/circleci/repo/node_modules/terser-webpack-plugin/dist/minify.js:8:38)
fs.js:115
    throw err;

Not sure that 1 and 2 are related however. Behaviour is not deterministic

Disabling terser-webpack-plugin (we use webpack) fixes both issues.

Our locked version is terser 3.14.

So:

  1. Should we downgrade to 3.12 ?
  2. What can we do to narrow the bug ?

Thank you :)

@eltonio450
Copy link

ok, so:

the webpack-terser-plugin automatically enables parallelism and caching. Disabling both fixes the issue on CircleCI if it helps :). Not sure if it's considered as a bug of terser or circleCI

@dylansmith
Copy link

@eltonio450 were you seeing exit code > 0 for this EFAULT issue? I'm also seeing it, it's impacting our production system, and the build passes 😱

@dylansmith
Copy link

Also, should add that we see ENOMEM in CircleCI before the EFAULT:

Error: ENOMEM: not enough memory, read
    at Object.readSync (fs.js:493:3)
    at tryReadSync (fs.js:332:20)
    at Object.readFileSync (fs.js:361:19)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:711:20)
    at Module.load (internal/modules/cjs/loader.js:600:32)
    at tryModuleLoad (internal/modules/cjs/loader.js:539:12)
    at Function.Module._load (internal/modules/cjs/loader.js:531:3)
    at Function.Module.runMain (internal/modules/cjs/loader.js:754:12)
    at startup (internal/bootstrap/node.js:283:19)
    at bootstrapNodeJSCore (internal/bootstrap/node.js:622:3)
fs.js:114
    throw err;
    ^

Error: EFAULT: bad address in system call argument, read
    at Object.readSync (fs.js:493:3)
    at tryReadSync (fs.js:332:20)
    at Object.readFileSync (fs.js:361:19)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:711:20)
    at Module.load (internal/modules/cjs/loader.js:600:32)
    at tryModuleLoad (internal/modules/cjs/loader.js:539:12)
    at Function.Module._load (internal/modules/cjs/loader.js:531:3)
    at Module.require (internal/modules/cjs/loader.js:637:17)
    at require (internal/modules/cjs/helpers.js:22:18)

@eltonio450
Copy link

i am not sure anymore, but the not enough memory seems to be some kind of timeout in our case. Did you try disabling parallelization and caching ?

@dylansmith
Copy link

Webpack was running 3 build configurations in parallel, each having a terser step. We tried reverting back to uglify and had similar issues, so this is probably not a terser issue specifically. We have switched to running the build configurations in series and this resolved the issue.

@fabiosantoscode
Copy link
Collaborator

Can this still be reproduced?

Reading this PR makes me believe this is a webpack thing (since they got around it by disabling parallelism): magda-io/magda#2233

@evilebottnawi have you fixed this on your end?

@alexander-akait
Copy link

Should be fixed, anyway speedup terser always is good, so if you can increase perf for terser, it will be awesome

@StefanSchoof
Copy link
Author

I started a build of my old branch and still seems not to finish: https://dev.azure.com/sschoof/espresso/_build/results?buildId=1350&view=logs&j=dd0cdf81-99cc-5b13-4144-b5aff478827b

@StefanSchoof
Copy link
Author

StefanSchoof commented Sep 28, 2019

The https://github.com/StefanSchoof/espresso/blob/terser/func/terser.js and https://github.com/StefanSchoof/espresso/blob/terser/func/input.js which can reproduce the problem without any webpack involved are still there

@fabiosantoscode
Copy link
Collaborator

I downloaded your files using wget and ran the build just fine (both on the latest master and 4.1.0). Maybe you just don't have enough RAM. That's a pretty big file. There's some discussion over at webpack-plugin-terser on making Terser only minify individual files instead of chunks, which can get really large.

@StefanSchoof
Copy link
Author

StefanSchoof commented Sep 29, 2019

Maybe I misunderstood you. The old problem is reproduceable with the old terser version (3.13) and without webpack. It was fixed by reverting 842392e in 3.14: #204 (comment)
Terser works since that without any issue. But I have no knowledge of the reverted change got back in again.

@fabiosantoscode
Copy link
Collaborator

Oh cool :) I thought it was back again, I didn't remember that got fixed with a revert and didn't read enough :) Thanks for letting me know anyway

@StefanSchoof
Copy link
Author

Your are welcome. Thank you for terser.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants