Bankim Bhavsar has uploaded this change for review. ( http://gerrit.cloudera.org:8080/15372
Change subject: [util] Add support for 32 & 64 byte alignment to Arena allocator ...................................................................... [util] Add support for 32 & 64 byte alignment to Arena allocator On adding couple of member variables and methods to BlockBloomFilter class for importing Or() function, predicate-test would sometimes randomly crash with SIGSEGV in AVX operations. On debugging, the crash would only happen when the "directory_" structure in BlockBloomFilter is not 32-bytes aligned. It's surprising without the addition of those methods, "directory_" so far has always been 32 bytes aligned despite Arena allocator not supporting alignment values greater than 16 bytes. With input from Todd, explored 3 options to fix the alignment problem: 1) Update the HeapBufferAllocator in util/memory to align allocation to 64 bytes. See AllocateInternal() implementation. Surprisingly the FLAGS_allocator_aligned_mode is OFF by default so we appear to be relying on the allocator to always allocate 16 byte aligned buffers. So this option would require turning ON the FLAGS_allocator_aligned_mode flag by default. 2) Update the Arena allocator such that when needed extra bytes are allocated to allow aligning with 32/64 bytes considering the new component will always be 16 byte aligned. This requires updating some address/alignment logic with offset_ and the new component allocation. 3) Don't touch the Arena allocator and simply add padding in the ArenaBlockBloomFilterBufferAllocator to minimize any risk to other parts of the codebase. Opted for option #2 since it broadly adds support for 32/64 byte alignment instead of limited scope of option #3. Option #1 is tempting but unsure about the unknowns that turning on the allocator_aligned_mode would bring. Although we need only support for 32 byte alignment for AVX operations, also added support for 64 bytes to better align with cache line size. Additionally this change: - Adds a simple BlockBloomFilter unit test that reproduced the alignment problem compared to end-to-end predicate-test which was turning out to be difficult to debug. - Fixes and enhances the arena-test with bunch of variations. Change-Id: Ib665115fa0fc262a8b76c48f52947dedb84be2a7 --- M src/kudu/util/block_bloom_filter-test.cc M src/kudu/util/block_bloom_filter.cc M src/kudu/util/memory/arena-test.cc M src/kudu/util/memory/arena.cc M src/kudu/util/memory/arena.h 5 files changed, 103 insertions(+), 36 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/72/15372/1 -- To view, visit http://gerrit.cloudera.org:8080/15372 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: Ib665115fa0fc262a8b76c48f52947dedb84be2a7 Gerrit-Change-Number: 15372 Gerrit-PatchSet: 1 Gerrit-Owner: Bankim Bhavsar <ban...@cloudera.com>
