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

[C++] bit_util: Remove the pre-computed kBitmask table ? #41687

Open
mapleFU opened this issue May 16, 2024 · 3 comments
Open

[C++] bit_util: Remove the pre-computed kBitmask table ? #41687

mapleFU opened this issue May 16, 2024 · 3 comments

Comments

@mapleFU
Copy link
Member

mapleFU commented May 16, 2024

Describe the enhancement requested

I see the pr removed the kBitMask in arrow-rs[1]. Besides, abseil mentions that this optimization is useless in Haswell machine [2][3], and this might also suffer from cache eviction. Should we trying to remove them and use something like above:

// Bitmask selecting the k-th bit in a byte
// static constexpr uint8_t kBitmask[] = {1, 2, 4, 8, 16, 32, 64, 128};
static constexpr uint8_t GetBitMask(uint8_t index) {
  // DCHECK(index >= 0 && index <= 7);
  return static_cast<uint8_t>(1) << index;
}

// the bitwise complement version of kBitmask
// static constexpr uint8_t kFlippedBitmask[] = {254, 253, 251, 247, 239, 223, 191, 127};
static constexpr uint8_t GetFlippedBitMask(uint8_t index) {
  // DCHECK(index >= 0 && index <= 7);
  return ~(static_cast<uint8_t>(1) << index);
}

[1] apache/arrow-rs#5771
[2] https://abseil.io/fast/9
[3] https://abseil.io/fast/39

Component(s)

C++

@mapleFU
Copy link
Member Author

mapleFU commented May 16, 2024

cc @felipecrv @pitrou

@pitrou
Copy link
Member

pitrou commented May 16, 2024

Well, feel free to do the change and run any benchmarks.

@mapleFU
Copy link
Member Author

mapleFU commented May 16, 2024

It's a bit faster on My M1 MacOS with existing benchmark ( arrow-bit-util-benchmark ), I'll push first and runing ursa benchmark

@felipecrv felipecrv changed the title [C++] bit_util: Remove the pre-computed kBitMask table ? [C++] bit_util: Remove the pre-computed kBitmask table ? May 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants