Openjdk hotspot build 14.0-b06 hard coded bug found.

[Tony Guan]

On Fri, Nov 14, 2008 at 4:58 PM, Jon Masamitsu <Jon.Masamitsu at sun.com> wrote:
> Tony Guan wrote On 11/14/08 14:12,:
>
>>Hi there,
>>
>>I think I've found a little bug in the parallel gc codes.  Will
>>somebody take a look at it?
>>Firstly, after a full gc, there maybe the need to adjust the boundary
>>between young and old generation.
>>and here in psMarkSweep.cpp, we
>>use:                   heap->resize_old_gen(size_policy->calculated_old_free_size_in_bytes());
>>
>>Then this methods will in turn call:
>>    gens()->adjust_boundary_for_old_gen_needs(desired_free_space);
>>
>>in this function, we compare the desired_free_space with the current
>>free space, and then calls request_old_gen_expansion:
>>    if (old_gen()->free_in_bytes() < desired_free_space) {
>>      MutexLocker x(ExpandHeap_lock);
>>      request_old_gen_expansion(desired_free_space);
>>but in request_old_gen_expansion the desired_free_space is immediately
>>treated as expand_in_bytes. And in this implementation, the actual
>>
>>
>
> Yes, you're right that this looks like a bug.
>
> Also something that doesn't look right here is the use of
> calculated_old_free_size_in_bytes().  It looks like it is adding
> room for 1 addtional young gen collection.  Perhaps not a
> bad idea but I don't recall now why it is used instead of the
> straight promo_size().
In my opinion, adding room for 1 additional young gen collection is
OK, because in adjustable parallel gc, by doing this, we can reduce
the chance of doing full gc for next time. And that's why it's called
desired_free_space.  (just a guess)


>
>
>>change in bytes is computed like this:
>> size_t change_in_bytes = MIN3(young_gen_available,
>>                                old_gen_available,
>>                                align_size_up_(expand_in_bytes, alignment));
>>and then:
>> virtual_spaces()->adjust_boundary_up(change_in_bytes)
>>
>>So in the end, we have old_gen()->free_in_bytes()+=desired_free_space
>>and the final "desired_free_space" is more than needed.
>>
>>And after the boundary is moved up (too high), the
>>_old_gen->resize(desired_free_space)
>>
>>will compute the new space size as:
>> size_t new_size = used_in_bytes() + desired_free_space;
>>
>>Thus it will shrink the old gen space smaller. So we have part the
>>space sacrificed by the young generation out of the old gen usage. And
>>the size of this wasted memory is (desired_free_space- prevoiusly free
>>space of old_gen).
>>
>>
>
> Yup, we've moved the boundary farther than we
> really wanted to.
>
> Are you able to file a bug report for this?  Did you want
> to fix it?

Is there any format should I follow to report the bug?
In fact it's simple to fix it. just change
in
void AdjoiningGenerations::adjust_boundary_for_old_gen_needs()
the following change will solve it:

-request_old_gen_expansion(desired_free_space);
+request_old_gen_expansion(desired_free_space-old_gen()->free_in_bytes());



>>
>>
>
>