From: Date: November 12 2007 12:44pm
Subject: bk commit into 5.1 tree (anozdrin:1.2608) BUG#31898
List-Archive: http://lists.mysql.com/commits/37569
X-Bug: 31898
Message-Id: <20071112114422.9C7E316476A@station.opbmk>

Below is the list of changes that have just been committed into a local
5.1 repository of alik. When alik does a push these changes will
be propagated to the main repository and, within 24 hours after the
push, to the public repository.
For information on how to access the public repository
see http://dev.mysql.com/doc/mysql/en/installing-source-tree.html

ChangeSet@stripped, 2007-11-12 14:44:17+03:00, anozdrin@station. +1 -0
  Fix for a BUG#31898: 16M memory allocations for user variables
  in stored procedure.
  
  The problem was that MySQL used unnecessarily large amounts of
  memory if user variables were used as an argument to CONCAT or
  CONCAT_WS -- 16M per each user variable used.
  
  Technically, it happened because MySQL used the following
  allocation strategy for string functions to avoid multiple
  realloc() calls: in the virtual operation fix_length_and_dec()
  the attribute max_length was calculated as a sum of max_length
  values for each argument.
  
  Although this approach worked well for small (or fixed) data types,
  there could be a problem if there as a user variable among
  the arguments of a string function -- max_length of the function
  would be 16M (as the max_length of a user variable is 16M).
  
  Both CONCAT() and CONCAT_WS() functions suffer from this problem.
  
  The fix is to do not use meta-data for allocating memory.
  The following strategy is proposed instead: allocate the exact
  length of the result string at the first record, double the amount
  of memory allocated when it is required.
  
  No test case for this bug because there is no way to test memory
  consumption in a robust way with our test suite.

  sql/item_strfunc.cc@stripped, 2007-11-12 14:44:15+03:00, anozdrin@station. +55 -5
    Implement memory-wise allocation strategy.

diff -Nrup a/sql/item_strfunc.cc b/sql/item_strfunc.cc
--- a/sql/item_strfunc.cc	2007-10-11 15:09:48 +04:00
+++ b/sql/item_strfunc.cc	2007-11-12 14:44:15 +03:00
@@ -356,10 +356,35 @@ String *Item_func_concat::val_str(String
       }
       else
       {						// Two big const strings
-	if (tmp_value.alloc(max_length) ||
-	    tmp_value.copy(*res) ||
-	    tmp_value.append(*res2))
+        /*
+          NOTE: We should be prudent in the initial allocation unit -- the
+          size of the arguments is a function of data distribution, which
+          can be any. Instead of overcommitting at the first row, we grow
+          the allocated amount by the factor of 2. This ensures that no
+          more than 25% of memory will be overcommitted on average.
+        */
+
+        uint concat_len= res->length() + res2->length();
+
+        if (tmp_value.alloced_length() < concat_len)
+        {
+          if (tmp_value.alloced_length() == 0)
+          {
+            if (tmp_value.alloc(concat_len))
+              goto null;
+          }
+          else
+          {
+            uint new_len = max(tmp_value.alloced_length() * 2, concat_len);
+
+            if (tmp_value.realloc(new_len))
+              goto null;
+          }
+        }
+
+	if (tmp_value.copy(*res) || tmp_value.append(*res2))
 	  goto null;
+
 	res= &tmp_value;
 	use_as_buff=str;
       }
@@ -679,8 +704,33 @@ String *Item_func_concat_ws::val_str(Str
     }
     else
     {						// Two big const strings
-      if (tmp_value.alloc(max_length) ||
-	  tmp_value.copy(*res) ||
+      /*
+        NOTE: We should be prudent in the initial allocation unit -- the
+        size of the arguments is a function of data distribution, which can
+        be any. Instead of overcommitting at the first row, we grow the
+        allocated amount by the factor of 2. This ensures that no more than
+        25% of memory will be overcommitted on average.
+      */
+
+      uint concat_len= res->length() + sep_str->length() + res2->length();
+
+      if (tmp_value.alloced_length() < concat_len)
+      {
+        if (tmp_value.alloced_length() == 0)
+        {
+          if (tmp_value.alloc(concat_len))
+            goto null;
+        }
+        else
+        {
+          uint new_len = max(tmp_value.alloced_length() * 2, concat_len);
+
+          if (tmp_value.realloc(new_len))
+            goto null;
+        }
+      }
+
+      if (tmp_value.copy(*res) ||
 	  tmp_value.append(*sep_str) ||
 	  tmp_value.append(*res2))
 	goto null;