-
-
Notifications
You must be signed in to change notification settings - Fork 34
fix(swc): Use fair/correct option #664
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
Conversation
It should, but due to a GitHub security limitation (preventing modifications to another repo from a different repo's GitHub Actions), it doesn't for external contributors. However, I ran it for you here: #666 You can find the raw stats in the I'll give this more thought. I previously rejected config changes from:
But I've previously made exceptions for Terser and Uglify since their compression modes produce significantly different results. The key issue is that there are endless configurations optimized for different inputs and we can only bench/compare so much. That said, most users just stick with the default settings, and I want these benchmarks to reflect the typical out-of-the-box experience. |
I'll investigate the reason for the difference. Actually, the |
I understand that you want to prevent the minifiers to game this benchmark by fine tuning the configs too much. But you should definitely make sure that every minifier removes unused top-level declarations, and also removes comments. Otherwise it's either incorrectly configured or there is a bug in the minifier (as kdy was saying). |
Disabling comments is definitely fine. Looking into the top-level declarations one. Some minifiers like esbuild simply doesn't support that: evanw/esbuild#3570 |
**Description:** I made a mistake. This PR also adds a size test for the `swc` crate, to ensure that the issue described in privatenumber/minification-benchmarks#664 is not our fault.
Closing to cleanup |
Were you able to reconcile the differences with https://github.com/swc-project/swc/blob/e862c329fc9af61414b664e81030976bde313983/crates/swc_ecma_minifier/scripts/size.sh ? |
Things I found:
fn gzip_size(data: &[u8]) -> usize {
use flate2::{write::GzEncoder, Compression};
use std::io::Write;
let mut encoder = GzEncoder::new(Vec::new(), Compression::best());
encoder.write_all(data).unwrap();
encoder.finish().unwrap().len()
} |
Guessing from the API of https://docs.rs/flate2/latest/flate2/write/index.html, I think zlib and gzip format is a different? As a side note, |
SWC has
size.sh
for printing the size of each file in this benchmark, but the result is very different. Does creating a PR will automatically run benchmarks?size.sh
: https://github.com/swc-project/swc/blob/e862c329fc9af61414b664e81030976bde313983/crates/swc_ecma_minifier/scripts/size.shThe minifier binary used by
size.sh
: https://github.com/swc-project/swc/blob/e862c329fc9af61414b664e81030976bde313983/crates/swc_ecma_minifier/examples/minifier.rsThis is the output of
size.sh
I removed
for brevity