impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Jim Apple (Code Review)" <ger...@cloudera.org>
Subject [Impala-ASF-CR] IMPALA-4123: Fast bit unpacking
Date Thu, 13 Oct 2016 17:58:20 GMT
Jim Apple has posted comments on this change.

Change subject: IMPALA-4123: Fast bit unpacking
......................................................................


Patch Set 10:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/4494/10/be/src/util/bit-packing-test.cc
File be/src/util/bit-packing-test.cc:

PS10, Line 39: it
them


PS10, Line 39: Both
Both buffers


Line 60:   uint8_t* packed = storage.data() + aligned;
If aligned is true, then packed is not aligned


http://gerrit.cloudera.org:8080/#/c/4494/10/be/src/util/bit-packing.inline.h
File be/src/util/bit-packing.inline.h:

Line 41: #define UNPACK_VALUES_CASE(ignore1, i, ignore2) \
Here is a way to do this without any macros, but it's a bit heavyweight from a syntax POV.
I found it was roughly the same speed as the macro approach:

    --- a/be/src/util/bit-packing.inline.h
    +++ b/be/src/util/bit-packing.inline.h
    @@ -31,10 +31,33 @@
     
     namespace impala {
     
    +template <typename T, int... I>
    +constexpr std::array<decltype(&T::template f<0>), sizeof...(I)> MakeArray(
    +    std::integer_sequence<int, I...>) {
    +  return {&T::template f<I>...};
    +}
    +
    +template <typename OutType>
    +struct BitPackStruct {
    +  template <int N>
    +  static auto f(const uint8_t* __restrict__ in, int64_t in_bytes, int64_t num_values,
    +      OutType* __restrict__ out) {
    +    return BitPacking::UnpackValues<OutType, N>(in, in_bytes, num_values, out);
    +  }
    +};
    +
     template <typename OutType>
     std::pair<const uint8_t*, int64_t> BitPacking::UnpackValues(int bit_width,
         const uint8_t* __restrict__ in, int64_t in_bytes, int64_t num_values,
         OutType* __restrict__ out) {
    +  static constexpr auto UNPACKERS =
    +      MakeArray<BitPackStruct<OutType>>(std::make_integer_sequence<int,
33>());
    +  if (bit_width < 0 || bit_width > 32) {
    +    DCHECK(false);
    +    return {nullptr, 0};
    +  }
    +  return UNPACKERS[bit_width](in, in_bytes, num_values, out);

Similar things can be done below. Line 146 can be done without std::make_integer_sequence.


-- 
To view, visit http://gerrit.cloudera.org:8080/4494
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I12db69409483d208cd4c0f41c27a78aeb6cd3622
Gerrit-PatchSet: 10
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong <tarmstrong@cloudera.com>
Gerrit-Reviewer: Jim Apple <jbapple@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <tarmstrong@cloudera.com>
Gerrit-HasComments: Yes

Mime
View raw message