Use leftmost region in GC allocations

[Dominik Inführ]

Yes, but AFAIU idx is of type size_t. So if _leftmost is 0, we would have
idx >= 0, which is always true for unsigned types and we would have an
endless loop.

Dominik

On Tue, Feb 6, 2018 at 9:02 PM, Aleksey Shipilev <shade at redhat.com> wrote:

> On 02/06/2018 08:05 PM, Dominik Inführ wrote:
> > Hi,
> >
> > allocate_single in ShenandoahFreeSet doesn't use the leftmost-Region.
> Quite
> > a minor problem but it might be worth to fix.
> >
> > diff --git a/src/hotspot/share/gc/shenandoah/shenandoahFreeSet.cpp
> > b/src/hotspot/share/gc/shenandoah/shenandoahFreeSet.cpp
> > --- a/src/hotspot/share/gc/shenandoah/shenandoahFreeSet.cpp
> > +++ b/src/hotspot/share/gc/shenandoah/shenandoahFreeSet.cpp
> > @@ -72,9 +72,9 @@
> >      }
> >      case ShenandoahHeap::_alloc_gclab:
> >      case ShenandoahHeap::_alloc_shared_gc: {
> > -      for (size_t idx = _rightmost; idx > _leftmost; idx--) {
> > -        if (is_free(idx)) {
> > -          HeapWord* result = try_allocate_in(word_size, type, idx,
> > in_new_region);
> > +      for (size_t idx = _rightmost + 1; idx > _leftmost; idx--) {
> > +        if (is_free(idx - 1)) {
> > +          HeapWord* result = try_allocate_in(word_size, type, idx - 1,
> > in_new_region);
> >            if (result != NULL) {
> >              return result;
> >            }
>
> Right.
>
> But isn't this just:
>
> $ hg diff
> diff -r a95aa48e56ed src/hotspot/share/gc/shenandoah/shenandoahFreeSet.cpp
> --- a/src/hotspot/share/gc/shenandoah/shenandoahFreeSet.cpp     Tue Feb
> 06 20:57:54 2018 +0100
> +++ b/src/hotspot/share/gc/shenandoah/shenandoahFreeSet.cpp     Tue Feb
> 06 21:00:13 2018 +0100
> @@ -72,7 +72,7 @@
>      }
>      case ShenandoahHeap::_alloc_gclab:
>      case ShenandoahHeap::_alloc_shared_gc: {
> -      for (size_t idx = _rightmost; idx > _leftmost; idx--) {
> +      for (size_t idx = _rightmost; idx >= _leftmost; idx--) {
>          if (is_free(idx)) {
>            HeapWord* result = try_allocate_in(word_size, type, idx,
> in_new_region);
>            if (result != NULL) {
>
> ?
>
> -Aleksey
>
>