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

Refactor the code of Bitmap::new #1337

Closed
HaoYang670 opened this issue Feb 18, 2022 · 1 comment · Fixed by #1343
Closed

Refactor the code of Bitmap::new #1337

HaoYang670 opened this issue Feb 18, 2022 · 1 comment · Fixed by #1343
Labels
arrow Changes to the arrow crate enhancement Any new improvement worthy of a entry in the changelog

Comments

@HaoYang670
Copy link
Contributor

Expected behavior
A clear and concise description of what you expected to happen.

    pub fn new(num_bits: usize) -> Self {
        let num_bytes = num_bits / 8 + if num_bits % 8 > 0 { 1 } else { 0 };
        let r = num_bytes % 64;
        let len = if r == 0 {
            num_bytes
        } else {
            num_bytes + 64 - r
        };
        Bitmap {
            bits: Buffer::from(&vec![0xFF; len]),
        }
    }

We can use bit_util::ceil and bit_util::round_upto_multiple_of_64 to refactor the code.

Additional context
Notice when working on #1230

@HaoYang670 HaoYang670 added the bug label Feb 18, 2022
@HaoYang670
Copy link
Contributor Author

Hope we could get a little performance improvement from it because faster arithmetical calculation is used:

pub fn round_upto_power_of_2(num: usize, factor: usize) -> usize {
    debug_assert!(factor > 0 && (factor & (factor - 1)) == 0);
    (num + (factor - 1)) & !(factor - 1)
}

sunchao pushed a commit that referenced this issue Feb 21, 2022
### Which issue does this PR close?

Closes #1337.

### Rationale for this change
 
`bit_util` has provided some functions to calculate the ceiling and multiple, so we can use them in `Bitmap::new` to achieve a faster and cleaner code.

### What changes are included in this PR?

### Are there any user-facing changes?

None.
@alamb alamb added arrow Changes to the arrow crate enhancement Any new improvement worthy of a entry in the changelog and removed bug labels Mar 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
arrow Changes to the arrow crate enhancement Any new improvement worthy of a entry in the changelog
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants