← Back to team overview

maria-developers team mailing list archive

Re: 405a89353b2: Don't reset StringBuffers in loops when not needed

 

Hi!

On Sat, Apr 3, 2021 at 12:54 AM Sergei Golubchik <serg@xxxxxxxxxxx> wrote:
>
> Hi, Monty!
>
> On Apr 02, Michael Widenius wrote:
> > revision-id: 405a89353b2 (mariadb-10.5.2-542-g405a89353b2)
> > parent(s): 2dd8d472fcc
> > author: Michael Widenius <michael.widenius@xxxxxxxxx>
> > committer: Michael Widenius <michael.widenius@xxxxxxxxx>
> > timestamp: 2021-03-24 19:25:08 +0200
> > message:
> >
> > Don't reset StringBuffers in loops when not needed
> >
> > - Moved out creating StringBuffers in loops and instead create them
> >   outside and just reset the buffer if it was not allocated (to avoid
> >   a possible malloc/free for every entry)
> >
> > diff --git a/sql/sql_string.h b/sql/sql_string.h
> > index 29d01779b71..8df50ac5e66 100644
> > --- a/sql/sql_string.h
> > +++ b/sql/sql_string.h
> > @@ -1045,6 +1045,12 @@ class StringBuffer : public String
> >    {
> >      length(0);
> >    }
> > +  void set_buffer_if_not_allocated(CHARSET_INFO *cs)
> > +  {
> > +    if (is_alloced())
> > +      Static_binary_string::set(buff, buff_sz);
>
> it seems to be doing exactly the opposite of what the name suggests.
> It sets the buffer *if allocated*.
>
> Also, it looks logical to do length(0) here, so that this method
> would reset the StringBuffer to its initial state.

What I have in my tree:
  /*
    This is used to set a new buffer for String.
    However if the String already has an allocated buffer, it will
    keep that one.
    It's not to be used to set the value or length of the string.
  */
  inline void set_buffer_if_not_allocated(char *str, size_t arg_length)
  {
    if (!alloced)
    {
      /*
        Following should really be str_length= 0, but some code may
        depend on that the String length is same as buffer length.
      */
      Ptr= str;
      str_length= Alloced_length= (uint32) arg_length;
    }
  }


References