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.

Sign up for GitHub

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

Jump to bottom

Make debug/tracing optional #18

Closed
gendx opened this issue Dec 10, 2019 · 0 comments · Fixed by #31
Closed

Make debug/tracing optional #18

gendx opened this issue Dec 10, 2019 · 0 comments · Fixed by #31
Labels
performance Make lzma-rs faster!

Comments

@gendx
Copy link
Owner

gendx commented Dec 10, 2019

The tracing macros such as info!, debug! or trace! from the log crate are not compiled away in release mode, because tracing can be enabled at runtime (via env_logger).

In principle, tracing can be disabled at compile time by enabling some features in the log crate (https://docs.rs/log/0.4.8/log/#compile-time-filters). However, enabling a feature for a crate affects all code that also depend on this crate within a build. So if a binary depends on lzma-rs and wants to log something else, then it cannot optimize away logging in lzma-rs via log's features.

A better solution would be to have a feature in lzma-rs to enable logging (as it is mostly useful for developing lzma-rs). By default, this logging feature will be off so that users of lzma-rs see a performance improvement in general.
@gendx gendx added the performance Make lzma-rs faster! label Dec 10, 2019
@gendx gendx mentioned this issue Mar 5, 2020
Merged
bors bot added a commit that referenced this issue Mar 5, 2020
@bors @gendx
Merge #31
c066d0e 
31: Gate logging behind an opt-in feature. r=gendx a=gendx

### Pull Request Overview

This pull request fixes #18. There is a new optional feature, `enable_logging` to turn on logging. When turned off (now default), the `lzma_trace!` and similar macros are no-ops.

This also removes the need for the `log` dependency (and all of its transitive dependencies) when turned off.


### Benchmarks

```
$ cargo bench --features enable_logging
running 8 tests
test compress_65536                  ... bench:   4,330,374 ns/iter (+/- 79,850)
test compress_empty                  ... bench:       2,221 ns/iter (+/- 370)
test compress_hello                  ... bench:       3,217 ns/iter (+/- 225)
test decompress_after_compress_65536 ... bench:   5,835,689 ns/iter (+/- 96,176)
test decompress_after_compress_empty ... bench:      10,845 ns/iter (+/- 553)
test decompress_after_compress_hello ... bench:      12,425 ns/iter (+/- 689)
test decompress_big_file             ... bench:   9,750,036 ns/iter (+/- 460,925)
test decompress_huge_dict            ... bench:      12,619 ns/iter (+/- 424)

$ cargo bench
running 8 tests
test compress_65536                  ... bench:   2,377,692 ns/iter (+/- 15,781)
test compress_empty                  ... bench:       2,067 ns/iter (+/- 64)
test compress_hello                  ... bench:       2,733 ns/iter (+/- 358)
test decompress_after_compress_65536 ... bench:   3,815,474 ns/iter (+/- 72,097)
test decompress_after_compress_empty ... bench:      10,760 ns/iter (+/- 5,275)
test decompress_after_compress_hello ... bench:      11,865 ns/iter (+/- 75)
test decompress_big_file             ... bench:   7,416,332 ns/iter (+/- 40,416)
test decompress_huge_dict            ... bench:      12,082 ns/iter (+/- 238)
```

Co-authored-by: G. Endignoux <ggendx@gmail.com>
@bors bors bot closed this as completed in 3fcf0bb Mar 5, 2020
@bors bors bot closed this as completed in #31 Mar 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
performance Make lzma-rs faster!
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Gate logging behind an opt-in feature. gendx/lzma-rs
1 participant
@gendx