RFR(S): 8007772: G1: assert(!hr->isHumongous() || mr.start() == hr->bottom()) failed: the start of HeapRegion and MemRegion should be consistent for humongous regions

[John Cuthbertson]

Hi Jesper,

On 2/11/2013 12:51 PM, Jesper Wilhelmsson wrote:
> I guess it's a matter of taste, to me it is a lot clearer. But sure, 
> it takes a second look at the code to see that short-circuiting 
> matters here.
>
> The version with code duplication is a bit confusing to me and 
> requires a second look to see that there is no difference between the 
> different clauses in the if/else if.
>
> I would argue that for someone who is new to the code, the 
> non-duplicated code would be easier to get familiar with due to (at a 
> first glance) less complicated code. And you don't have to know that 
> the short-circuiting is necessary until you know the code well enough 
> to realize that it is necessary, if you see what I mean.
>
> We could also add a comment :-)
> /Jesper
>

We could argue but I don't think that you will convince me that your 
suggestion is clearer. I first read the code the same way as Bengt and 
was going to make a similar comment as his until an "Aha!" moment and I 
saw the short-circuit. Reading the code incorrectly has biased me. :)

Thanks,

JohnC