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

The problem was that when one used String::alloc() to allocate a string,
the String ensures that there is space for an extra NULL byte in the
buffer and if not, reallocates the string. This is a problem with the
String::set_int() that calls alloc(21), which forces extra
malloc/free calls to happen.

- We do not anymore re-allocate String if alloc() is called with the
  Allocated_length. This reduces number of malloc() allocations,
  especially one big re-allocation in Protocol::send_result_Set_metadata()
  for almost every query that produced a result to the connnected client.
- Avoid extra mallocs when using LONGLONG_BUFFER_SIZE
  This can now be done as alloc() doesn't increase buffers if new length is
  not bigger than old one.
- c_ptr() is redesigned to be safer (but a bit longer) than before.
- Remove wrong usage of c_ptr_quick()
  c_ptr_quick() was used in many cases to get the pointer to the used
  buffer, even when it didn't need to be \0 terminated. In this case
  ptr() is a better substitute.
  Another problem with c_ptr_quick() is that it did not guarantee that
  the string would be \0 terminated.
- item_val_str(), an API function not used currently by the server,
  now always returns a null terminated string (before it didn't always
  do that).
- Ensure that all String allocations uses STRING_PSI_MEMORY_KEY. The old
  mixed usage of performance keys caused assert's when String buffers
  where shrunk.
- Binary_string::shrink() is simplifed
- Fixed bug in String(const char *str, size_t len, CHARSET_INFO *cs) that
  used Binary_string((char *) str, len) instead of Binary_string(str,len).
- Changed argument to String() creations and String.set() functions to use
  'const char*' instead of 'char*'. This ensures that Alloced_length is
  not set, which gives safety against someone trying to change the
  original string. This also would allow us to use !Alloced_length in
  c_ptr() if needed.
- Changed string_ptr_cmp() to use memcmp() instead of c_ptr() to avoid
  a possible malloc during string comparision.

1 files changed, 51 insertions, 12 deletions

diff --git a/sql/sql_string.h b/sql/sql_string.h
index b3eca118b63..c731f6189c8 100644
--- a/sql/sql_string.h
+++ b/sql/sql_string.h
@@ -600,25 +600,55 @@ public:
 
   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) and
+      String("hello",5) efficiently.
+
+      We have two options here. To test for !Alloced_length or !alloced.
+      Using "Alloced_length" is slightly safer so that we do not read
+      from potentially unintialized memory (normally not dangerous but
+      may give warnings in valgrind), but "alloced" is safer as there
+      are less change to get memory loss from code that is using
+      String((char*), length) or String.set((char*), length) and does
+      not free things properly (and there is several places in the code
+      where this happens and it is hard to find out if any of these will call
+      c_ptr().
+    */
+    if (unlikely(!alloced && !Ptr[str_length]))
+      return Ptr;
+    if (str_length < Alloced_length)
+    {
+      Ptr[str_length]=0;
+      return Ptr;
+    }
+    (void) realloc(str_length+1);               /* This will add end \0 */
     return Ptr;
   }
+  /*
+    One should use c_ptr() instead for most cases. This will be deleted soon,
+    kept for compatiblity.
+  */
   inline char *c_ptr_quick()
   {
-    if (Ptr && str_length < Alloced_length)
-      Ptr[str_length]=0;
-    return Ptr;
+    return c_ptr_safe();
   }
+  /*
+    This is to be used only in the case when one cannot use c_ptr().
+    The cases are:
+    - When one initializes String with an external buffer and length and
+      buffer[length] could be uninitalized when c_ptr() is called.
+    - When valgrind gives warnings about uninitialized memory with c_ptr().
+  */
   inline char *c_ptr_safe()
   {
     if (Ptr && str_length < Alloced_length)
       Ptr[str_length]=0;
     else
-      (void) realloc(str_length);
+      (void) realloc(str_length + 1);
     return Ptr;
   }
 
@@ -634,7 +664,16 @@ public:
   }
   inline bool alloc(size_t arg_length)
   {
-    if (arg_length < Alloced_length)
+    /*
+      Allocate if we need more space or if we don't have p_done any
+      allocation yet (we don't want to have Ptr to be NULL for empty strings).
+
+      Note that if arg_length == Alloced_length then we don't allocate.
+      This ensures we don't do any extra allocations in protocol and String:int,
+      but the string will not be atomically null terminated if c_ptr() is not
+      called.
+    */
+    if (arg_length <= Alloced_length && Alloced_length)
       return 0;
     return real_alloc(arg_length);
   }
@@ -642,7 +681,7 @@ public:
   bool realloc_raw(size_t arg_length);
   bool realloc(size_t arg_length)
   {
-    if (realloc_raw(arg_length))
+    if (realloc_raw(arg_length+1))
       return TRUE;
     Ptr[arg_length]= 0; // This make other funcs shorter
     return FALSE;
@@ -743,7 +782,7 @@ public:
   */
   String(const char *str, size_t len, CHARSET_INFO *cs)
    :Charset(cs),
-    Binary_string((char *) str, len)
+    Binary_string(str, len)
   { }
   String(char *str, size_t len, CHARSET_INFO *cs)
    :Charset(cs),