facebook / facebook/folly

ConcurrentHashMap allocates unaligned memory for SegmentT which alignas 64

Open
#1,844 3 comments 7 reactions 0 assignees View on GitHub
Dominant language
C++
Stars
30.5k
Forks
5.9k
PR merge metrics
No merged PRs in 30d

Description

# Bugs
ConcurrentHashMap allocates memory by std::allocator, which won't generate memory aligned at 64B.
This has caused coredump when I compiled with `clang14` and `-mavx2`:

**Clang14**
![image](https://user-images.githubusercontent.com/57653828/186337551-4632c836-ca55-435d-ad85-027b35d255da.png)

**Clang14 IR**
![image](https://user-images.githubusercontent.com/57653828/186337688-91a2a724-b964-4465-beb9-172061bd8685.png)

In clang14, mutex will view as aligned at 64B, and `vmovaps` instruction will be used to optimize.

## reproduce codes
I've copied some codes from folly to reproduce this bugs. folly version is `v2018.08.20.00`. This example won't coredump, but we can found it uses `vmovaps` from objdump and the reason from clang IR.

I'm not sure it is still a problem in main branch.

```shell
# clang IR
/path/to/clang++14 -S -emit-llvm -std=c++14 -O3 -mavx2 main.cpp
```
```c++
#include
#include
#include
#include
#include

template <
typename KeyType,
typename ValueType,
uint8_t ShardBits = 8,
typename HashFn = std::hash,
typename KeyEqual = std::equal_to,
typename Allocator = std::allocator,
template class Atom = std::atomic,
class Mutex = std::mutex>
class alignas(64) ConcurrentHashMapSegment {

public:
ConcurrentHashMapSegment(
size_t initial_buckets,
float load_factor,
size_t max_size)
: load_factor_(load_factor), max_size_(max_size) {
std::cout << "initial_buckets: " << initial_buckets << std::endl;
}

private:
Mutex m_;
float load_factor_;
size_t const max_size_;
};

int main() {

using SegmentT = ConcurrentHashMapSegment;
using Allocator = std::allocator;

SegmentT* newseg = (SegmentT*)Allocator().allocate(sizeof(SegmentT));
// SegmentT* newseg = new SegmentT(16, 1.0, 16);
printf("%p\n", newseg);
newseg = new (newseg)
SegmentT(16, 1.0, 16);

}
```

Contributor guide

Open the contributing guide

Research direction

Start by reproducing the issue with the provided Clang 14 command, -mavx2 flags, and sample ConcurrentHashMapSegment allocation. Inspect how ConcurrentHashMap allocates SegmentT and compare the resulting alignment with the alignas(64) requirement; done means the allocation is valid for SegmentT and the reproduced alignment problem no longer occurs.

Written by the indexing model from the issue text.

Assessment

Tech stack
cpp
Domain
performance
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Needs clarification
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.