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
_______________________________________________
Mailing list: https://launchpad.net/~maria-developers
Post to : maria-developers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~maria-developers
More help : https://help.launchpad.net/ListHelp
