Small improvements to SkBitSet:

Small improvements to SkBitSet:
  - implement O(1) operations in SkBitSet.h so they can inline away
  - use calloc to allocate empty bitsets instead of malloc then clear
  - little style things

BUG=skia:

Committed: https://skia.googlesource.com/skia/+/e099dd73dff0f447029323e918118a189bb27679

[mtklein, 2014-10-09 15:41:12]

mtklein@google.com changed reviewers:
+ mtklein@google.com, reed@google.com

[mtklein, 2014-10-09 15:41:34]

The CQ bit was checked by mtklein@google.com

[commit-bot: I haz the power, 2014-10-09 15:43:05]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/640243003/1

[commit-bot: I haz the power, 2014-10-09 15:43:07]

Note for Reviewers:
The CQ is waiting for an approval.
If you believe that the CL is not ready yet, or if you would like to L-G-T-M
with comments then please uncheck the CQ checkbox.

Waiting for LGTM from valid reviewer(s) till 2014-10-09 21:43 UTC

[reed1, 2014-10-09 15:53:42]

reed@google.com changed reviewers:
+ caryclark@google.com

[reed1, 2014-10-09 15:53:43]

https://codereview.chromium.org/640243003/diff/1/src/utils/SkBitSet.h
File src/utils/SkBitSet.h (right):

https://codereview.chromium.org/640243003/diff/1/src/utils/SkBitSet.h#newcode34
src/utils/SkBitSet.h:34: uint32_t mask = 1 << (index % 32);
nit: in skia using % 32 is far more rare than (index & 0x1F)

its also possible, since index is signed, that the two are not equivalent (in
terms of perf).

https://codereview.chromium.org/640243003/diff/1/src/utils/SkBitSet.h#newcode36
src/utils/SkBitSet.h:36: *(this->internalGet(index)) |= mask;
stylistic:

uint32_t mask = ...
uint32_t* chunk = this->internalGet(index);
if (value) {
    *chunk |= mask;
} else {
    *chunk &= ~mask;
}

I just get the hebejebees seeing the same function called twice, even when I
assume that the compiler will likely only call it once...

https://codereview.chromium.org/640243003/diff/1/src/utils/SkBitSet.h#newcode46
src/utils/SkBitSet.h:46: return 0 != (*this->internalGet(index) & mask);
trivial nit: use SkToBool(...) instead?

[mtklein, 2014-10-09 16:12:23]

The CQ bit was checked by mtklein@google.com

[commit-bot: I haz the power, 2014-10-09 16:12:25]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/640243003/20001

[commit-bot: I haz the power, 2014-10-09 16:12:25]

Note for Reviewers:
The CQ is waiting for an approval.
If you believe that the CL is not ready yet, or if you would like to L-G-T-M
with comments then please uncheck the CQ checkbox.

Waiting for LGTM from valid reviewer(s) till 2014-10-09 22:12 UTC

[mtklein, 2014-10-09 16:13:00]

https://codereview.chromium.org/640243003/diff/1/src/utils/SkBitSet.h
File src/utils/SkBitSet.h (right):

https://codereview.chromium.org/640243003/diff/1/src/utils/SkBitSet.h#newcode34
src/utils/SkBitSet.h:34: uint32_t mask = 1 << (index % 32);
On 2014/10/09 15:53:43, reed1 wrote:
> nit: in skia using % 32 is far more rare than (index & 0x1F)
> 
> its also possible, since index is signed, that the two are not equivalent (in
> terms of perf).

Done.

Generated code looks fine to me both ways, but might as well use a mask. 
Probably was inlining helping here, effectively letting setBit know index is
actually unsigned.

https://codereview.chromium.org/640243003/diff/1/src/utils/SkBitSet.h#newcode36
src/utils/SkBitSet.h:36: *(this->internalGet(index)) |= mask;
On 2014/10/09 15:53:43, reed1 wrote:
> stylistic:
> 
> uint32_t mask = ...
> uint32_t* chunk = this->internalGet(index);
> if (value) {
>     *chunk |= mask;
> } else {
>     *chunk &= ~mask;
> }
> 
> I just get the hebejebees seeing the same function called twice, even when I
> assume that the compiler will likely only call it once...

Done.

For what it's worth, it had better only call it once.  They're on opposite sides
of an if-statement.  (And now that it's inlined, if we call this with a
compile-time constant value, the whole if-statement compiles away.)

https://codereview.chromium.org/640243003/diff/1/src/utils/SkBitSet.h#newcode46
src/utils/SkBitSet.h:46: return 0 != (*this->internalGet(index) & mask);
On 2014/10/09 15:53:43, reed1 wrote:
> trivial nit: use SkToBool(...) instead?

Done.

[reed1, 2014-10-09 18:48:48]

lgtm

[commit-bot: I haz the power, 2014-10-09 18:49:33]

Committed patchset #2 (id:20001) as e099dd73dff0f447029323e918118a189bb27679