RFR: 8155043: BitMap set operations assume clear bits beyond unaligned end

[Jon Masamitsu]

On 8/14/2016 4:23 PM, Kim Barrett wrote:
>> On Aug 12, 2016, at 5:06 PM, Jon Masamitsu <jon.masamitsu at oracle.com> wrote:
>>
>> Kim,
>>
>> This patch is good to go as is but I just have some questions.
> Thanks.
>
>> Did you give any consideration to just having set_union_with_result()
>> and using it where set_union() is used (ignoring the return value)?
> I wouldn't think the added cost of tracking whether the value was
> changed would be considered a good thing when it isn't needed.  If the
> function were inlined the compiler could optimize away the change
> tracking, but it's not.

I might have traded any reasonable additional cost for the removal of
code that is very similar but your choice is fine.

>
>> I note in the test parameters with double underscore in the names.
>>
>> 162 TEST(BitMap, is_full_or_empty__aligned)
>>
>> I can see how it aids readability but it seems new.  Is it new
>> and did you decide it was a convention worth using?
> Since we've only recently started using gtest, pretty much any usage
> or convention is new.  We don't presently seem to have any "official"
> conventions around test naming; at least, I wasn't able to discern any
> from the existing tests, and I haven't found any document providing
> guidance.  That should probably be fixed.
>
> I'm not at all wedded to the namings I used. I picked them because
> they need to be globally unique and they seemed to help me with
> readability.

I agree it is more readable.   Would you bring it up at the next staff 
meeting
to see if there is any concerns?

Thanks.

Jon

>> As I said, good to go as is.
>>
>> Jon
>>
>> On 8/11/2016 4:48 PM, Kim Barrett wrote:
>>>> On Aug 9, 2016, at 4:31 AM, Stefan Karlsson <stefan.karlsson at oracle.com> wrote:
>>>>
>>>> On 2016-08-08 20:56, Kim Barrett wrote:
>>>>>> On Aug 5, 2016, at 9:27 AM, Stefan Karlsson <stefan.karlsson at oracle.com> wrote:
>>>>>>
>>>>>> Hi Kim,
>>>>>>
>>>>>> This looks mostly good to me. I have a few comments inlined:
>>>>> Thanks for looking at this.
>>>>>
>>>>> New webrevs:
>>>>> full: http://cr.openjdk.java.net/~kbarrett/8155043/webrev.01/
>>>>> incr: http://cr.openjdk.java.net/~kbarrett/8155043/webrev.01.inc/
>>>> Reviewed.
>>> Thanks.
>>>
>>> Any other reviewers?
>