parquet::SetArrayBit

["Katelman, Michael" <Mi...@CubistSystematic.com>]

Hi,

Sorry for posting this here and not in Jira (factors outside my control necessitate me posting here). Maybe my interpretation of the semantics of parquet::SetArrayBit is off, but I was surprised to find SetArrayBit(bits, i, false) does not clear bit i. I just wanted to mention it in case that wasn't the behavior that was intended and is a bug.

from bit-util.h
  static inline void SetArrayBit(uint8_t* bits, int i, bool is_set) {
    bits[i / 8] |= (1 << (i % 8)) * is_set;
  }





DISCLAIMER: This e-mail message and any attachments are intended solely for the use of the individual or entity to which it is addressed and may contain information that is confidential or legally privileged. If you are not the intended recipient, you are hereby notified that any dissemination, distribution, copying or other use of this message or its attachments is strictly prohibited. If you have received this message in error, please notify the sender immediately and permanently delete this message and any attachments.

RE: parquet::SetArrayBit

["Katelman, Michael" <Mi...@CubistSystematic.com>]

Thanks for letting me know. I just wanted to make sure. re: performance, I agree it's hard to say. The conditional statement version may also turn into a cmov and avoid branch mispredictions.

-----Original Message-----
From: Wes McKinney [mailto:wesmckinn@gmail.com] 
Sent: Monday, June 26, 2017 11:59
To: dev@parquet.apache.org
Subject: Re: parquet::SetArrayBit

Seems that function could use some documentation. It is not intended to be able to clear bits, but rather to set a bit to 1 only if is_set is true.
Another way would be

if (is_set) {
  bits[i / 8] |= 1 << (i % 8);
}

In theory the branch-free version may be faster, but I have not run any benchmarks to check

On Mon, Jun 26, 2017 at 8:55 AM, Katelman, Michael < Michael.Katelman@cubistsystematic.com> wrote:

> Hi,
>
> Sorry for posting this here and not in Jira (factors outside my 
> control necessitate me posting here). Maybe my interpretation of the 
> semantics of parquet::SetArrayBit is off, but I was surprised to find 
> SetArrayBit(bits, i, false) does not clear bit i. I just wanted to 
> mention it in case that wasn't the behavior that was intended and is a bug.
>
> from bit-util.h
>   static inline void SetArrayBit(uint8_t* bits, int i, bool is_set) {
>     bits[i / 8] |= (1 << (i % 8)) * is_set;
>   }
>
>
>
>
>
> DISCLAIMER: This e-mail message and any attachments are intended 
> solely for the use of the individual or entity to which it is 
> addressed and may contain information that is confidential or legally 
> privileged. If you are not the intended recipient, you are hereby 
> notified that any dissemination, distribution, copying or other use of 
> this message or its attachments is strictly prohibited. If you have 
> received this message in error, please notify the sender immediately 
> and permanently delete this message and any attachments.
>
>
>
>





DISCLAIMER: This e-mail message and any attachments are intended solely for the use of the individual or entity to which it is addressed and may contain information that is confidential or legally privileged. If you are not the intended recipient, you are hereby notified that any dissemination, distribution, copying or other use of this message or its attachments is strictly prohibited. If you have received this message in error, please notify the sender immediately and permanently delete this message and any attachments.

Re: parquet::SetArrayBit

[Wes McKinney <we...@gmail.com>]

Seems that function could use some documentation. It is not intended to be
able to clear bits, but rather to set a bit to 1 only if is_set is true.
Another way would be

if (is_set) {
  bits[i / 8] |= 1 << (i % 8);
}

In theory the branch-free version may be faster, but I have not run any
benchmarks to check

On Mon, Jun 26, 2017 at 8:55 AM, Katelman, Michael <
Michael.Katelman@cubistsystematic.com> wrote:

> Hi,
>
> Sorry for posting this here and not in Jira (factors outside my control
> necessitate me posting here). Maybe my interpretation of the semantics of
> parquet::SetArrayBit is off, but I was surprised to find SetArrayBit(bits,
> i, false) does not clear bit i. I just wanted to mention it in case that
> wasn't the behavior that was intended and is a bug.
>
> from bit-util.h
>   static inline void SetArrayBit(uint8_t* bits, int i, bool is_set) {
>     bits[i / 8] |= (1 << (i % 8)) * is_set;
>   }
>
>
>
>
>
> DISCLAIMER: This e-mail message and any attachments are intended solely
> for the use of the individual or entity to which it is addressed and may
> contain information that is confidential or legally privileged. If you are
> not the intended recipient, you are hereby notified that any dissemination,
> distribution, copying or other use of this message or its attachments is
> strictly prohibited. If you have received this message in error, please
> notify the sender immediately and permanently delete this message and any
> attachments.
>
>
>
>