← Back to team overview

maria-developers team mailing list archive

Re: f46382ba93c: Optimize usage of c_ptr(), c_ptr_quick() and String::alloc()

 

Hi!

Sorry, this was because gmail send it before I wanted to.
(I do not like that ctrl+return sends email as this causes problems
when one uses this a lot in slack!)

<cut>
> > > --- a/sql/sql_string.h
> > > +++ b/sql/sql_string.h
> > > @@ -600,25 +600,34 @@ class Binary_string: public Static_binary_string
> > >
> > >    inline char *c_ptr()
> > >    {
> > > -    DBUG_ASSERT(!alloced || !Ptr || !Alloced_length ||
> > > -                (Alloced_length >= (str_length + 1)));
> > > -
> > > -    if (!Ptr || Ptr[str_length])              // Should be safe
> > > -      (void) realloc(str_length);
> > > +    if (unlikely(!Ptr))
> > > +      return (char*) "";
> > > +    /*
> > > +      Here we assume that any buffer used to initalize String has
> > > +      an end \0 or have at least an accessable character at end.
> > > +      This is to handle the case of String("Hello",5) efficently.
> > > +    */
> > > +    if (unlikely(!alloced && !Ptr[str_length]))
> > > +      return Ptr;
> >
> > No, this is not good. Note the difference between
> >
> >   String a("Hello", 5)
> >
> > and
> >
> >   char hello[5];
> >   String a(buf, 5);
> >
> > Your assumption should only apply to the first case, not to the second.
> > In  the first case alloced=Alloced_length=0, in the second case only
> > alloced=0 and Alloced_length=5. So in the if() above you need to look
> > at Alloced_length:
> >
> >   if (!Alloced_length && !Ptr[str_length])
> >     return Ptr;

Unfortunately this does not work good with our code.

We have a lot of code that does things like this:
 String *new_str= new (thd->mem_root) String((const char*) name.str,
                                              name.length,
                                              system_charset_info)
Where string is 0 terminated.
With your proposed change, all of these will require a full realloc when using
c_ptr().

Regards,
Monty


Follow ups

References