[
https://issues.apache.org/jira/browse/ORC-445?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16710732#comment-16710732
]
ASF GitHub Bot commented on ORC-445:
------------------------------------
fangzheng commented on a change in pull request #346: ORC-445: [C++] Code
improvements in RLEV2Util.
URL: https://github.com/apache/orc/pull/346#discussion_r239274880
##########
File path: c++/src/RLEV2Util.hh
##########
@@ -23,85 +23,32 @@
namespace orc {
extern const uint32_t FBSToBitWidthMap[FixedBitSizes::SIZE];
+ extern const uint32_t ClosestFixedBitsMap[65];
+ extern const uint32_t ClosestAlignedFixedBitsMap[65];
+ extern const uint32_t BitWidthToFBSMap[65];
inline uint32_t decodeBitWidth(uint32_t n) {
return FBSToBitWidthMap[n];
Review comment:
Hi Gang, I thought this over and don't feel we need to add boundary check
here for two reasons:
1. in current RLEv2 encoder and decoder code, all the callers of
decodeBitWidth() pass in a integer that only has the lowest 5 bits set, so the
input n is always less than FixedBitSizes::SIZE (32).
2. Since this function is used to decode the 5-bit length code, it would be
a programming error for a caller to pass in any value that is >= 32. In this
case, returning 64 (as the original implementation does) is not helping. That
would just silently mask the error (potentially file content corruption) and is
likely to cause further error when decoding with the wrong bit width.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> [C++] Code improvements in RLEV2Util
> ------------------------------------
>
> Key: ORC-445
> URL: https://issues.apache.org/jira/browse/ORC-445
> Project: ORC
> Issue Type: Improvement
> Components: C++
> Reporter: Fang Zheng
> Priority: Minor
>
> This is a follow-up of ORC-444. The following functions in RLEV2Util.hh can
> be optimized by replacing the if-else statements with direct array lookup:
> inline uint32_t getClosestFixedBits(uint32_t n);
> inline uint32_t getClosestAlignedFixedBits(uint32_t n);
> inline uint32_t encodeBitWidth(uint32_t n);
> inline uint32_t findClosestNumBits(int64_t value);
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
