maria-developers team mailing list archive

[Sergei Golubchik <serg@xxxxxxxxxxx>]

Hi, Sergey!

On Mar 19, Sergey Vojtovich wrote:
> > > === modified file 'sql/sql_plist.h'
> > > --- a/sql/sql_plist.h	2013-11-20 11:05:39 +0000
> > > +++ b/sql/sql_plist.h	2014-03-15 18:53:01 +0000
> > > @@ -142,6 +142,14 @@ class I_P_List : public C, public I
> > >  
> > >      return result;
> > >    }
> > > +  inline T *back()
> > > +  {
> > > +    T *result= m_first;
> > > +    if (result)
> > > +      while (*B::next_ptr(result))
> > > +        result= *B::next_ptr(result);
> > > +    return result;
> > > +  }
> > 
> > I think this is wrong. The correct way would be to use I_P_List<> with
> > the I_P_List_fast_push_back policy, then you could use
> > have get_last() method to get the last element in the list.
> > May be as
> > 
> >     inline T *back()
> >     {
> >       return *I::get_last();
> >     }
> > 
> I was considering I_P_List_fast_push_back, but I had reasons not to use it:
> 1. It'll have to maitain m_last (last element pointer), which involves extra
>    shared memory location write. Even though there are good chances for m_head
>    and m_last to share the same cache line. This maintenance will happen on
>    a hot path.
> 
> 2. I'm aiming to replace I_P_List with single-linked list (updated atomically).
>    I can remove this ugly back() implementation when I'm done, or alternatively
>    I can move it now to table_cache.cc.
> 
> 3. We nerver use back() on a hot path, I believe there is not much meaning to
>    sacrifice hot path performance.

Okay. In that case, please rename the method to back_slow() or something
that would prevent it being mindlessly used everywhere. Something that
would make one to to stop and think "hmm, this is an expensive method,
do I really want to use it here?"

Optionally, you can also add this simple back() as above, so that one
would have a fast alternative.

Or, indeed, you can remove back() implementation from I_P_List and move
it to table_cache.cc, if that's possible.

I'm ok with either approach.

Regards,
Sergei