You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Bankim Bhavsar (Code Review)" <ge...@cloudera.org> on 2020/04/24 01:41:16 UTC

[kudu-CR] [util] Don't use static function ptrs for member functions in BlockBloom filter

Hello Alexey Serbin, Kudu Jenkins, Andrew Wong, Volodymyr Verovkin, Wenzhe Zhou, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/15767

to look at the new patch set (#2).

Change subject: [util] Don't use static function ptrs for member functions in BlockBloom filter
......................................................................

[util] Don't use static function ptrs for member functions in BlockBloom filter

Use of static function pointers to non-static member functions causes
crash in Kudu client in Impala test cases.

Backtrace from an instrumented build that checks for null function pointer:
https://gist.github.com/bbhavsar/1580e5e897dcd7271bad623f7da631f0

I don't understand the problem completely but it seems that static POD like
std::once_flag doesn't get destructed but static function pointers that point
to non-static member functions and are initialized just once
point to nullptr across test runs which leads to NPE.

I couldn't reproduce the problem in Kudu using similar combination of Bloom
filter predicate and range predicate values.

OrEqual functions are static whereas Insert/Find are non-static.
So this change:
- Uses non-static member function pointers for Insert/Find that are initialized
in constructor(this is same as the change before introduction of OrEqual
functions) avoiding the branch in Insert/Find.
- Doesn't use a static function pointer for OrEqual functions and checks for
AVX2 capability at run-time on every invocation.

Tests:
- No crashes with the same test run in Impala with the fix.

Change-Id: I39703fd1a7e256ff60ef86d0b370590fbb526380
---
M src/kudu/util/block_bloom_filter-test.cc
M src/kudu/util/block_bloom_filter.cc
M src/kudu/util/block_bloom_filter.h
3 files changed, 42 insertions(+), 50 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/67/15767/2
-- 
To view, visit http://gerrit.cloudera.org:8080/15767
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I39703fd1a7e256ff60ef86d0b370590fbb526380
Gerrit-Change-Number: 15767
Gerrit-PatchSet: 2
Gerrit-Owner: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Volodymyr Verovkin <ve...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>