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

subscriber: disable regex dependency default features #899

Merged
merged 1 commit into from
Aug 7, 2020

Conversation

bdonlan
Copy link
Contributor

@bdonlan bdonlan commented Aug 7, 2020

The regex dependency is used only to parse configuration directives, and
therefore is not performance critical. This change turns off all default
features (except 'std') from regex in order to optimize for compile times for
this non-performance-critical code.

Note that due to the presence of the 'criterion' dependency (which has its own
default-features dependency on regex), the tests do not normally run with only
the minimal set of regex features. I did remove the criterion dependency to
verify that tests build, but I'm not sure how to test that we're not using
disabled regex features on an ongoing basis.

Motivation

Solution

The regex dependency is used only to parse configuration directives, and
therefore is not performance critical.  This change turns off all default
features (except 'std') from regex in order to optimize for compile times for
this non-performance-critical code.

Note that due to the presence of the 'criterion' dependency (which has its own
default-features dependency on regex), the tests do not normally run with only
the minimal set of regex features. I did remove the criterion dependency to
verify that tests build, but I'm not sure how to test that we're not using
disabled regex features on an ongoing basis.
@bdonlan bdonlan requested review from hawkw and a team as code owners August 7, 2020 21:41
@hawkw
Copy link
Member

hawkw commented Aug 7, 2020

Note that due to the presence of the 'criterion' dependency (which has its own
default-features dependency on regex), the tests do not normally run with only
the minimal set of regex features.

This is probably not a problem, as long as downstream users don't have to compile those features. It's fine to enable them just when compiling tests, as the dependency tree is likely much shallower anyway.

@hawkw hawkw merged commit 7f24283 into tokio-rs:master Aug 7, 2020
hawkw added a commit that referenced this pull request Aug 11, 2020
Fixed

- **env-filter**: Incorrect max level hint when filters involving span
  field values are in use (#907)
- **registry**: Fixed inconsistent span stacks when multiple registries
  are in use on the same thread (#901)

Changed

- **env-filter**: `regex` dependency enables fewer unused feature flags
  (#899)

Thanks to @bdonlan and @jeromegn for contributing to this release!
hawkw added a commit that referenced this pull request Aug 11, 2020
### Fixed

- **env-filter**: Incorrect max level hint when filters involving span
  field values are in use (#907)
- **registry**: Fixed inconsistent span stacks when multiple registries
  are in use on the same thread (#901)

### Changed

- **env-filter**: `regex` dependency enables fewer unused feature flags
  (#899)

Thanks to @bdonlan and @jeromegn for contributing to this release!
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.

2 participants