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

Including chrono-tz for simple usage results in binary bloat #39

Closed
PhilipDaniels opened this issue Jan 12, 2020 · 6 comments
Closed

Including chrono-tz for simple usage results in binary bloat #39

PhilipDaniels opened this issue Jan 12, 2020 · 6 comments

Comments

@PhilipDaniels
Copy link
Contributor

I'm using chrono-tz to convert from London times to UTC, for which it is great. (Spent several hours googling and was not able to figure out how to do this using just chrono). However, this is the only thing I need to do, it's literally one line in my app. I noticed that adding chrono-tz to my Cargo.toml doubled the size of the exe (I'm on Windows).

This is the Cargo.toml

chrono = { version = "0.4", features = ["serde"] }
chrono-tz = "0.5"
encoding = "0.2"
env_logger = "0.7"
log = "0.4"
logging_timer = "0.9.2"
num-traits = "0.2"
num-derive = "0.3"
once_cell = "1.2"
prettytable-rs = "0.8.0"
regex = "1.3"
rust_decimal = "1.1"

EXE size without chrono-tz is 1,121,280 bytes.
WITH chrono-tz and it rises to 2,281,984 bytes.

I haven't delved into how chrono-tz works but this seems like a large hit for one line of code. Is it including the entire TZ database?

It would be nice (and I stress nice, this is obviously by no means a show-stopper issue) to have some way of slimming this down...perhaps by restructuring so that the compiler can do a better job of eliminating unused code, or if it's down to the database perhaps some way of indicating which timezones one is interested in?

@AlyoshaVasilieva
Copy link
Contributor

Try adding this to your Cargo.toml:

[profile.release]
lto = true
codegen-units = 1

It may significantly slow down compile time in release mode but should also decrease binary size. I'm using chrono-tz on a microcontroller and it adds maybe 40KB to the binary (only timezones in use are UTC and my directly-specified local timezone).

@quodlibetor
Copy link
Contributor

We could probably pretty easily add an environment variable or config file that would allow filtering timezones by a regex or some other pattern.

We could filter the table after we've constructed it, using some collection of patterns. I'm also happy to extend Table to have some extra features to make this easier.

I don't love the idea of having builds be different depending on the presence of an env var, and if I'm reading this cargo issue correctly it seems like there's no way to have downstream crates set an env var from a config file. I also can't find a way to get the build dir of the final binary from a build script.

I'd take a PR adding filtering based on an env var, but if there's a better way to do it I'd rather do it better.

@PhilipDaniels
Copy link
Contributor Author

@AlyoshaVasilieva I've tried that and I still see a hit of about 1MB. Looking at the generated timezones.rs, for it to have any effect we would need rustc to optimize away unused enum variants and match branches, which I don't think it does. So I can't explain your result.

@quodlibetor My initial thought was this would be doable using cargo features, but alas they seem quite limited, there's no way to pass parameters such as a pattern to them. It does look like the usual way of passing parameters to build scripts is to use environment variables. It does seem a bit icky though, it would be much nicer to specify something in the Cargo.toml so that everybody could continue to just do a cargo build as normal.

I think the way I would do it is have an environment variable CHRONO_TZ_BUILD_TIMEZONES whose value is either empty, giving current behaviour, or it's a regex specifying which timezones to include, for example, "GMT|US/.*|Europe/London".
There's a thing you can put in build.rs to watch for changes to an environment variable.

I'll take a stab at this, it might be a few days before you see anything though, I'm in the middle of another Rust project at the moment :-)

@PhilipDaniels
Copy link
Contributor Author

Actually there is a thing called cargo package metadata which might allow something of the form

[package.metadata.chrono-tz]
timezones = "GMT|US/.*|Europe/London"

It might then be possible to get at this using the cargo_metadata crate.
Not sure though, documentation seems sketchy around this. Exactly which Cargo.toml is involved would be crucial (my binary's, or chrono-tz's own?).

@quodlibetor
Copy link
Contributor

Exactly which Cargo.toml is involved would be crucial (my binary's, or chrono-tz's own?

Not only this, but it seems hard to determine what to do if multiple libraries all specify this. It does seem like something that should only be specified by the final build, which I suppose is an argument for an env var.

@quodlibetor
Copy link
Contributor

I believe that the closes cargo issue to make this nice instead of depending on env vars is rust-lang/cargo#6373

@djc djc closed this as not planned Won't fix, can't repro, duplicate, stale Apr 5, 2024
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 a pull request may close this issue.

5 participants