RFR (S) 8211926: Catastrophic size_t underflow in BitMap::*_large methods

[Kim Barrett]

> On Nov 12, 2018, at 2:14 AM, Thomas Stüfe <thomas.stuefe at gmail.com> wrote:
> 
> Hi Kim,
> 
> On Sat, Nov 10, 2018 at 6:55 PM Kim Barrett <kim.barrett at oracle.com> wrote:
>> 
>>> On Nov 10, 2018, at 7:58 AM, Thomas Stüfe <thomas.stuefe at gmail.com> wrote:
>>> - I find it confusing that we use the same type (idx_t) both for word-
>>> and bit-indices. It would make the code a bit more readable if those
>>> were separate types (even if they both were to alias to size_t,
>>> ultimately).
>> 
>> I dislike this too.  I’d be interested in seeing a proposal to fix that, though
>> I worry about the fanout.  I *think* clients mostly operate in bits and the
>> use of words is mostly limited to the implementation, but I haven’t done
>> the searches needed to check that.
> 
> Should I have the time I'll give this a try. Not sure when I will find
> the time though.

Same here.

>>> - Generally, I find putting the decision between "large" and
>>> "non-large" APIs off to the caller a bit strange. As a user of bitmap,
>>> I cannot know at which point which variant would be better. I would
>>> prefer the class itself handling that.
>> 
>> I was thinking much the same thing while reviewing this change.
>> Maybe some of the public API in this area should be revisited.
>> In particular, I was surprised that one might call “large” functions
>> directly, rather than calling hinted functions with a “large” hint.
>> Perhaps the “large” functions should be private helpers that just
>> do what they are told (maybe with an assert as before), and the
>> calls to them should be based on the hint and the runtime size
>> check they are now performing.  Or maybe the whole hint thing
>> should be reconsidered.  I’d be interested in a proposal in this
>> area too.
> 
> I would consider switching between loops and memset to be an internal
> tuning parameter which the caller should not have to bother with. I
> think hinting makes sense when I know something the code cannot know.
> But there are no hidden information here beside the range size. Unless
> I misunderstand the problem.

I mostly agree.  The thing a hint could be used for is to order a sequence
of tests.  For example, if hinted large, then check for large first, while if
hinted small, check for the single bit or all bits in one word cases first.