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, 27 insertions, 27 deletions

diff --git a/sql/sql_string.cc b/sql/sql_string.cc
index f2a0f55aec8..aca9ede9000 100644
--- a/sql/sql_string.cc
+++ b/sql/sql_string.cc
@@ -41,7 +41,7 @@ bool Binary_string::real_alloc(size_t length)
   if (Alloced_length < arg_length)
   {
     free();
-    if (!(Ptr=(char*) my_malloc(PSI_INSTRUMENT_ME,
+    if (!(Ptr=(char*) my_malloc(STRING_PSI_MEMORY_KEY,
                                 arg_length,MYF(MY_WME | (thread_specific ?
                                                 MY_THREAD_SPECIFIC : 0)))))
       return TRUE;
@@ -55,7 +55,8 @@ bool Binary_string::real_alloc(size_t length)
 
 
 /**
-   Allocates a new buffer on the heap for this String.
+   Allocates a new buffer on the heap for this String if current buffer is
+   smaller.
 
    - If the String's internal buffer is privately owned and heap allocated,
      one of the following is performed.
@@ -70,7 +71,8 @@ bool Binary_string::real_alloc(size_t length)
      will be allocated and the string copied accoring to its length, as found
      in String::length().
  
-   For C compatibility, the new string buffer is null terminated.
+   For C compatibility, the new string buffer is null terminated if it was
+   allocated.
 
    @param alloc_length The requested string size in characters, excluding any
    null terminator.
@@ -81,9 +83,10 @@ bool Binary_string::real_alloc(size_t length)
 
    @retval true An error occurred when attempting to allocate memory.
 */
+
 bool Binary_string::realloc_raw(size_t alloc_length)
 {
-  if (Alloced_length <= alloc_length)
+  if (Alloced_length < alloc_length)
   {
     char *new_ptr;
     uint32 len= ALIGN_SIZE(alloc_length+1);
@@ -92,13 +95,13 @@ bool Binary_string::realloc_raw(size_t alloc_length)
       return TRUE;                                 /* Overflow */
     if (alloced)
     {
-      if (!(new_ptr= (char*) my_realloc(PSI_INSTRUMENT_ME, Ptr,len,
+      if (!(new_ptr= (char*) my_realloc(STRING_PSI_MEMORY_KEY, Ptr,len,
                                         MYF(MY_WME |
                                             (thread_specific ?
                                              MY_THREAD_SPECIFIC : 0)))))
         return TRUE;				// Signal error
     }
-    else if ((new_ptr= (char*) my_malloc(PSI_INSTRUMENT_ME, len,
+    else if ((new_ptr= (char*) my_malloc(STRING_PSI_MEMORY_KEY, len,
                                          MYF(MY_WME |
                                              (thread_specific ?
                                               MY_THREAD_SPECIFIC : 0)))))
@@ -118,9 +121,14 @@ bool Binary_string::realloc_raw(size_t alloc_length)
   return FALSE;
 }
 
+
 bool String::set_int(longlong num, bool unsigned_flag, CHARSET_INFO *cs)
 {
-  uint l=20*cs->mbmaxlen+1;
+  /*
+    This allocates a few bytes extra in the unlikely case that cs->mb_maxlen
+    > 1, but we can live with that
+  */
+  uint l= LONGLONG_BUFFER_SIZE * cs->mbmaxlen;
   int base= unsigned_flag ? 10 : -10;
 
   if (alloc(l))
@@ -235,7 +243,7 @@ bool Binary_string::copy()
 */
 bool Binary_string::copy(const Binary_string &str)
 {
-  if (alloc(str.str_length))
+  if (alloc(str.str_length+1))
     return TRUE;
   if ((str_length=str.str_length))
     bmove(Ptr,str.Ptr,str_length);		// May be overlapping
@@ -246,7 +254,7 @@ bool Binary_string::copy(const Binary_string &str)
 bool Binary_string::copy(const char *str, size_t arg_length)
 {
   DBUG_ASSERT(arg_length < UINT_MAX32);
-  if (alloc(arg_length))
+  if (alloc(arg_length+1))
     return TRUE;
   if (Ptr == str && arg_length == uint32(str_length))
   {
@@ -272,7 +280,7 @@ bool Binary_string::copy(const char *str, size_t arg_length)
 bool Binary_string::copy_or_move(const char *str, size_t arg_length)
 {
   DBUG_ASSERT(arg_length < UINT_MAX32);
-  if (alloc(arg_length))
+  if (alloc(arg_length+1))
     return TRUE;
   if ((str_length=uint32(arg_length)))
     memmove(Ptr,str,arg_length);
@@ -1251,24 +1259,16 @@ bool String::append_semi_hex(const char *s, uint len, CHARSET_INFO *cs)
   return false;
 }
 
+
 // Shrink the buffer, but only if it is allocated on the heap.
 void Binary_string::shrink(size_t arg_length)
 {
-    if (!is_alloced())
-        return;
-    if (ALIGN_SIZE(arg_length + 1) < Alloced_length)
-    {
-        char* new_ptr;
-        if (!(new_ptr = (char*)my_realloc(STRING_PSI_MEMORY_KEY, Ptr, arg_length,
-            MYF(thread_specific ? MY_THREAD_SPECIFIC : 0))))
-        {
-            Alloced_length = 0;
-            real_alloc(arg_length);
-        }
-        else
-        {
-            Ptr = new_ptr;
-            Alloced_length = (uint32)arg_length;
-        }
-    }
+  if (is_alloced() && ALIGN_SIZE(arg_length + 1) < Alloced_length)
+  {
+    /* my_realloc() can't fail as new buffer is less than the original one */
+    Ptr= (char*) my_realloc(STRING_PSI_MEMORY_KEY, Ptr, arg_length,
+                            MYF(thread_specific ?
+                                MY_THREAD_SPECIFIC : 0));
+    Alloced_length= (uint32) arg_length;
+  }
 }