RFR(XS): 8214235: assertion in collectedHeap.cpp: attempt to clean empty remainder

[Boris Ulasevich]

Hello Paul,

Thank for your input! I do not see how my final fix can introduce 
non-object gap (may be you had a look to webrev.01 which was rejected?):

 > http://cr.openjdk.java.net/~bulasevich/8214235/webrev.00
 > void ThreadLocalAllocBuffer::insert_filler() {
 >   assert(end() != NULL, "Must not be retired");
 > + if (top() < hard_end()) {
 >     Universe::heap()->fill_with_dummy_object(top(), hard_end(), true);
 > + }
 > }

I checked ">=" change with my crasher. It works OK (assertion check does 
not fire because TLAB alignment_reserve grows 0->2 and therefore top 
never hits hard_end). Do you think I should proceed (test, review) with 
this fix instead of mine?

thanks,
Boris

On 27.12.2018 20:13, Hohensee, Paul wrote:
> Hi Boris,
> 
> I don't think this is the actual bug, rather it's in collectedHeap.cpp
> 
> size_t CollectedHeap::tlab_alloc_reserve() const {
>    size_t min_size = min_dummy_object_size();
>    return min_size > (size_t)MinObjAlignment ? align_object_size(min_size) : 0;
> }
> 
> ">" should be ">=". Would you try it out on your crasher please?
> 
> During GC, an invariant is that eden (and other spaces) be  "parseable", i.e., it's filled with legal objects from bottom to top. Your patch can introduce a non-object gap in eden at arbitrary places.
> 
> Thanks,
> 
> Paul
> 
> On 12/18/18, 7:14 AM, "hotspot-gc-dev on behalf of Per Liden" <hotspot-gc-dev-bounces at openjdk.java.net on behalf of per.liden at oracle.com> wrote:
> 
>      Hi Boris,
>      
>      On 12/11/18 3:13 PM, Boris Ulasevich wrote:
>      > Hi Per,
>      >
>      >    You asked if I have reproducer for this on x86. I do have! The
>      > configuration [--with-jvm-variants=client --with-debug-level=fastdebug]
>      > on 32bit linux-x86 (alignment_reserve is empty for this configuration)
>      > fires the assertion even on build time:
>      > #  Internal Error
>      > (/ws/boris/jdk-jdk/src/hotspot/share/gc/shared/collectedHeap.cpp:373),
>      > pid=4338, tid=4339
>      > #  assert(words >= min_fill_size()) failed: too small to fill
>      >
>      > My fix for the issue works Ok:
>      >    http://cr.openjdk.java.net/~bulasevich/8214235/webrev.00
>      
>      Thanks for the reproducer, I think the above patch looks good.
>      
>      cheers,
>      Per
>      
>      >    http://bugs.openjdk.java.net/browse/JDK-8214235
>      >
>      > Boris
>      >
>      > On 29.11.2018 18:13, Boris Ulasevich wrote:
>      >> Hi Per,
>      >>
>      >> On 29.11.2018 16:30, Per Liden wrote:
>      >>> Hi,
>      >>>
>      >>> On 11/29/18 12:02 PM, Boris Ulasevich wrote:
>      >>>> Hi Per,
>      >>>>
>      >>>> On 28.11.2018 17:35, Per Liden wrote:
>      >>>>> Hi,
>      >>>>>
>      >>>>> On 11/28/18 1:39 PM, Boris Ulasevich wrote:
>      >>>>>> Hi all,
>      >>>>>>
>      >>>>>> Please review a simple fix: do not fill remaining tlab memory
>      >>>>>> when the remainder is empty.
>      >>>>>
>      >>>>> Hmm I get the feeling something else is wrong here. Like a
>      >>>>> misaligned object or a misaligned object size was allocated in the
>      >>>>> TLAB.
>      >>>>
>      >>>> Actually assertion check catches the case when top() = end().
>      >>>> I do not think it is something criminal. May be it is better to
>      >>>> add check (top() < hard_end()) instead:
>      >>>> http://cr.openjdk.java.net/~bulasevich/8214235/webrev.00
>      >>>
>      >>> There's still something fishy here. The TLAB alignment_reserve should
>      >>> make sure there's always enough space left for a filler object.
>      >>
>      >> My case is different! alignment_reserve() returns 0:
>      >> alignment_reserve() = MAX2(tlab_alloc_reserve(),
>      >> _reserve_for_allocation_prefetch) =
>      >> MAX2(min_dummy_object_size() > MinObjAlignment ?
>      >> min_dummy_object_size() : 0, is_server_compilation_mode_vm() ? a+b*c/d
>      >> : 0) =
>      >> MAX2(2 > 2 ? 2 : 0, 0 ? a+b*c/d : 0) = MAX2(0, 0) = 0
>      >>
>      >>> Do you have a reproducer for this on x86 or does this only happen on
>      >>> Arm?
>      >>>
>      >>> /Per
>      >>>
>      >>>>
>      >>>>> When you crash here it would be interesting to see what the tlab
>      >>>>> start/end/top/etc is and what MinObjAlignement is being used.
>      >>>>
>      >>>> TLAB: start()=0x938b0b28, end()=0x938dcdc8, top()=0x938dcdc8
>      >>>> MinObjAlignment: 2
>      >>>>
>      >>>>> cheers,
>      >>>>> Per
>      >>>>>
>      >>>>>>
>      >>>>>> https://bugs.openjdk.java.net/browse/JDK-8214235
>      >>>>>> http://cr.openjdk.java.net/~bulasevich/8214235/webrev.01
>      >>>>>>
>      >>>>>> Thanks,
>      >>>>>> Boris
>      
>