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

Reduce packaged crate size by 5kb using cargo diet -r #983

Merged
merged 4 commits into from May 31, 2020

Conversation

Byron
Copy link
Contributor

@Byron Byron commented May 28, 2020

It converted excludes into includes, removing the following files
from the package.

Since this solely optimizes for size, I am happy to make the necessary adjustments to add other files back that you want to be part of the package.

┌───────────────────────────────────────────┬─────────────┐
│ File                                      │ Size (Byte) │
├───────────────────────────────────────────┼─────────────┤
│ .gitignore                                │          77 │
│ .github/ISSUE_TEMPLATE/other.md           │          80 │
│ .github/ISSUE_TEMPLATE/feature_request.md │         274 │
│ .github/ISSUE_TEMPLATE/compile-issue.md   │         488 │
│ utils/ci/install_cargo_web.sh             │         536 │
│ COPYRIGHT                                 │         569 │
│ utils/ci/miri.sh                          │         815 │
│ rustfmt.toml                              │         863 │
│ benches/weighted.rs                       │        1088 │
│ utils/ci/install.sh                       │        1381 │
│ utils/ci/script.sh                        │        1583 │
│ examples/monte-carlo.rs                   │        1611 │
│ appveyor.yml                              │        2098 │
│ SECURITY.md                               │        2822 │
│ .travis.yml                               │        2952 │
│ utils/ziggurat_tables.py                  │        3929 │
│ examples/monty-hall.rs                    │        4004 │
│ benches/misc.rs                           │        4524 │
│ benches/seq.rs                            │        5410 │
│ benches/generators.rs                     │        5490 │
└───────────────────────────────────────────┴─────────────┘

It converted excludes into includes, removing the following files
from the package.

┌───────────────────────────────────────────┬─────────────┐
│ File                                      │ Size (Byte) │
├───────────────────────────────────────────┼─────────────┤
│ .gitignore                                │          77 │
│ .github/ISSUE_TEMPLATE/other.md           │          80 │
│ .github/ISSUE_TEMPLATE/feature_request.md │         274 │
│ .github/ISSUE_TEMPLATE/compile-issue.md   │         488 │
│ utils/ci/install_cargo_web.sh             │         536 │
│ COPYRIGHT                                 │         569 │
│ utils/ci/miri.sh                          │         815 │
│ rustfmt.toml                              │         863 │
│ benches/weighted.rs                       │        1088 │
│ utils/ci/install.sh                       │        1381 │
│ utils/ci/script.sh                        │        1583 │
│ examples/monte-carlo.rs                   │        1611 │
│ appveyor.yml                              │        2098 │
│ SECURITY.md                               │        2822 │
│ .travis.yml                               │        2952 │
│ utils/ziggurat_tables.py                  │        3929 │
│ examples/monty-hall.rs                    │        4004 │
│ benches/misc.rs                           │        4524 │
│ benches/seq.rs                            │        5410 │
│ benches/generators.rs                     │        5490 │
└───────────────────────────────────────────┴─────────────┘
Copy link
Collaborator

@vks vks left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel like COPYRIGHT should be included as well. Removing the other files seems reasonable to me.

Copy link
Member

@dhardy dhardy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about Cargo.toml itself; is this implicitly included? Yes (according to doc).

Then this looks good to me.

Could you do the same for rand_distr please (this has benches too)?

@Byron
Copy link
Contributor Author

Byron commented May 29, 2020

Thanks for the hint, it's done for rand_distr as well.

Cargo.toml Outdated
autobenches = true
edition = "2018"
include = ["src/**/*", "LICENSE-*", "README.md", "CHANGELOG.md", "COPYRIGHT"]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't we just match src here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good! I have made it 'src/' which has the same effect but makes clearer that it's a directory.
If that's not alright, please let me know and I will make it src as originally requested.

Copy link
Member

@dhardy dhardy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@dhardy dhardy merged commit 2c2fbd6 into rust-random:master May 31, 2020
@Byron
Copy link
Contributor Author

Byron commented Jun 1, 2020

Thank you! Now we are at 35 million downloads all time, and when 70 million are reached, this change will have saved roughly 35 million * 5kb = 175gigabytes of downstream bandwidth. That must count for something :).

@dhardy
Copy link
Member

dhardy commented Jun 1, 2020

Well, not quite — it takes a while to get a new version out, and even longer to get everyone updated.

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

3 participants