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

speed up --reduce-test #3726

Merged
merged 1 commit into from
Feb 17, 2020
Merged

speed up --reduce-test #3726

merged 1 commit into from
Feb 17, 2020

Conversation

alexlamsl
Copy link
Collaborator

  • avoid pathological test case branches via adaptive time-out
  • use initial test case elapsed time to adjust maximum time-out
  • index output cache using hash instead of raw source

@alexlamsl alexlamsl force-pushed the reduce-test branch 3 times, most recently from 484a57a to 032d026 Compare February 17, 2020 14:45
- avoid pathological test case branches via adaptive time-out
- use initial test case elapsed time to adjust maximum time-out
@alexlamsl alexlamsl merged commit 53517db into mishoo:master Feb 17, 2020
@alexlamsl alexlamsl deleted the reduce-test branch February 17, 2020 15:35
@kzc
Copy link
Contributor

kzc commented Feb 17, 2020

Good stuff.

XOR bug enthusiasts like me will save a lot of time. :-)

$ time bin/uglifyjs -c --reduce-test <<< 'var a=0;do{}while(a||(3^3));console.log(a);'
// reduce test pass 1, iteration 0: 44 bytes
// reduce test pass 1, iteration 25: 39 bytes
// reduce test pass 1: 24 bytes
// reduce test pass 2: 16 bytes
// reduce test pass 3: 16 bytes
do {
    0;
} while (3 ^ 3);
// output: 
// minify: Error: Script execution timed out.
// options: {
//   "compress": true,
//   "mangle": false
// }

real	0m11.011s
user	0m11.004s
sys	0m0.052s

Now that pathological cases have been dealt with, you might consider increasing the number of passes from 3 to 4 or higher so that the larger test cases like the one you pointed out earlier today could be further reduced. Perhaps test case minified size (> 250 bytes) and total run time (under 20 seconds) can be taken into account before running further passes.

@kzc
Copy link
Contributor

kzc commented Feb 17, 2020

Wait, never mind - that won't work... I didn't realize that Node 10 cannot reduce this test case further with more passes:
#3725 (comment)

@kzc
Copy link
Contributor

kzc commented Feb 17, 2020

It would be useful to tack on an extra comment at the end of the reduced test case showing the Node process.version needed to reproduce the problem.

@alexlamsl
Copy link
Collaborator Author

Grrr... need to update all those mocha tests for that extra line of comment 🤣

@alexlamsl alexlamsl mentioned this pull request Feb 17, 2020
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

Successfully merging this pull request may close these issues.

None yet

2 participants