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

Kim Barrett kim.barrett at oracle.com
Sun Aug 14 23:23:43 UTC 2016


> 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 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.

> 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?




More information about the hotspot-dev mailing list