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

Disable default features for rocksdb #416

Merged
merged 1 commit into from
Aug 4, 2021

Conversation

tcharding
Copy link
Contributor

@tcharding tcharding commented Aug 2, 2021

Description

In an effort to reduce the build times of rocksdb we can set
default-features to false.

Please note, the build speed up is minimil

With default features:

cargo check --features compact_filters  890.91s user 47.62s system 352% cpu 4:26.55 total

Without default features:

cargo check --features compact_filters  827.07s user 47.63s system 352% cpu 4:08.39 total

Enable snappy since it seems like this is the current default compression
algorithm, therefore this patch (hopefully) makes no changes to the usage of the
rocksdb library in bdk. From the rocksdb code:

    /// Sets the compression algorithm that will be used for compressing blocks.
    ///
    /// Default: `DBCompressionType::Snappy` (`DBCompressionType::None` if
    /// snappy feature is not enabled).
    ///
    /// # Examples
    ///
    /// ```
    /// use rocksdb::{Options, DBCompressionType};
    ///
    /// let mut opts = Options::default();
    /// opts.set_compression_type(DBCompressionType::Snappy);
    /// ```
    pub fn set_compression_type(&mut self, t: DBCompressionType) {
        ....

Notes to the reviewers

Done in an attempt to help with #244

Checklists

All Submissions:

  • I've signed all my commits
  • I followed the contribution guidelines
  • I ran cargo fmt and cargo clippy before committing

New Features:

  • I've added tests for the new feature
  • I've added docs for the new feature
  • I've updated CHANGELOG.md

Bugfixes:

  • This pull request breaks the existing API
  • I've added tests to reproduce the issue which are now passing
  • I'm linking the issue being fixed by this PR

Copy link
Member

@RCasatta RCasatta left a comment

Choose a reason for hiding this comment

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

LGTM

for rocksdb:
default = ["snappy", "lz4", "zstd", "zlib", "bzip2"]
it appears we don't need the other compression algorithms

@tcharding
Copy link
Contributor Author

LGTM

for rocksdb:
default = ["snappy", "lz4", "zstd", "zlib", "bzip2"]
it appears we don't need the other compression algorithms

That was my guess also, I have zero experience with rocksdb. At first I tried to build with no features enabled and that builds just fine but from the docs I posted above I'm guessing that this means no compression which I guess we don't want.

Thanks for the review!

@afilini
Copy link
Member

afilini commented Aug 3, 2021

I think this needs a rebase before we can merge it, because in master there are now two more CI jobs

In an effort to reduce the build times of `rocksdb` we can set
`default-features` to false.

Please note, the build speed up is minimil

With default features:
```
cargo check --features compact_filters  890.91s user 47.62s system 352% cpu 4:26.55 total
```

Without default features:
```
cargo check --features compact_filters  827.07s user 47.63s system 352% cpu 4:08.39 total
```

Enable `snappy` since it seems like this is the current default compression
algorithm, therefore this patch (hopefully) makes no changes to the usage of the
`rocksdb` library in `bdk`. From the `rocksdb` code:

```
    /// Sets the compression algorithm that will be used for compressing blocks.
    ///
    /// Default: `DBCompressionType::Snappy` (`DBCompressionType::None` if
    /// snappy feature is not enabled).
    ///
    /// # Examples
    ///
    /// ```
    /// use rocksdb::{Options, DBCompressionType};
    ///
    /// let mut opts = Options::default();
    /// opts.set_compression_type(DBCompressionType::Snappy);
    /// ```
    pub fn set_compression_type(&mut self, t: DBCompressionType) {
        ....
```
@afilini afilini merged commit e80be49 into bitcoindevkit:master Aug 4, 2021
@tcharding tcharding deleted the rocksdb branch August 17, 2021 06:08
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