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

Improve build time by turning off regex cargo features #6

Merged
merged 2 commits into from
Dec 20, 2019

Conversation

bluetech
Copy link
Contributor

Note: I realized after I finished that this PR will increase the minimum required Rust version to that of regex 1.3.1, which is 1.28.0. This is probably not acceptable at this time. But thought it better to send the PR anyway, for reference.


The first commit adds a benchmark example in order to measure the impact of the second commit.


Second commit:

Only enable the regex features we need. See here for reference:
https://docs.rs/regex/1.3.1/regex/#crate-features

This reduces compilation time in release mode on my computer from ~30s
to ~16s (~50% decrease).

This increases the benchmark runtime on my computer in release mode with
-C target-cpu=native from ~0.5s to ~0.65s (~%30 increase).

chrono-tz uses this crate at build time to parse a few files, in which
case runtime speed is not super important, while compilation speed is.

If a crate does require maximum speed, it can enable regex's perf
feature in its own Cargo.toml.

Only enable the regex features we need. See here for reference:
https://docs.rs/regex/1.3.1/regex/#crate-features

This reduces compilation time in release mode on my computer from ~30s
to ~16s (~50% decrease).

This increases the benchmark runtime on my computer in release mode with
`-C target-cpu=native` from ~0.5s to ~0.65s (~%30 increase).

chrono-tz uses this crate at build time to parse a few files, in which
case runtime speed is not super important, while compilation speed is.

If a crate does require maximum speed, it can enable regex's `perf`
feature in their own Cargo.toml.
@ammgws
Copy link

ammgws commented Dec 18, 2019

@quodlibetor Are there any stoppers to merging this one?

@quodlibetor
Copy link
Contributor

ah, no I just missed it! I'll be able to merge it tomorrow, I think.

@6A61736F6E206E61646572
Copy link

6A61736F6E206E61646572 commented Dec 19, 2019

Good to hear!

Do the new versions of the crates (this and chrono-tz if parse-zoneinfo is bumped to 0.3 ) have to be manually uploaded, or is it automated?

@quodlibetor quodlibetor merged commit ee8e671 into chronotope:master Dec 20, 2019
@ammgws
Copy link

ammgws commented Dec 21, 2019

@quodlibetor Do we need to bump the version too?

@quodlibetor
Copy link
Contributor

@ammgws yup, done.

@djzin master branch is ready for publishing as 0.2.1 whenever you have time.

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.

4 participants