List:Commits« Previous MessageNext Message »
From:Evgeny Potemkin Date:November 6 2009 7:34pm
Subject:bzr commit into mysql-5.1-bugteam branch (epotemkin:3200) Bug#34384
View as plain text  
#At file:///work/bzrroot/34384-bug-5.1-bugteam/ based on revid:joro@stripped

 3200 Evgeny Potemkin	2009-11-06
      Bug#34384: Slow down on constant conversion.
      When values of different types are compared they're converted to a type that
      allows correct comparison. This conversion is done for each comparison and
      takes some time. When a constant is being compared it's possible to cache the
      value after conversion to speedup comparison. In some cases (large dataset,
      complex WHERE condition with many type conversions) query might be executed
      7% faster.
      
      A test case isn't provided because all changes are internal and isn't visible
      outside.
      
      The behavior of the Item_cache is changed to cache values on the first request
      of cached value rather than at the moment of storing item to be cached.
      A flag named value_cached is added to the Item_cache class. It's set to TRUE
      when cache holds the value of the last stored item.
      Function named cache_value() is added to the Item_cache class and derived classes.
      This function actually caches the value of the saved item.
      Item_cache_xxx::store functions now only store item to be cached and set
      value_cached flag to FALSE.
      Item_cache_xxx::val_xxx functions are changed to call cache_value function
      prior to returning cached value if value_cached is FALSE.
      The Arg_comparator::set_cmp_func function now calls cache_converted_constant
      to cache constants if they need a type conversion.
      The Item_cache::get_cache function is overloaded to allow setting of the
      cache type.
      The cache_converted_constant function is added to the Arg_comparator class.
      It checks whether a value can and should be cached and if so caches it.
     @ sql/item.cc
        Bug#34384: Slow down on constant conversion.
        Function named cache_value() is added to the Item_cache class and derived classes.
        This function actually caches the value of the saved item.
        Item_cache_xxx::store functions now only store item to be cached and set
        value_cached flag to FALSE.
        Item_cache_xxx::val_xxx functions are changed to call cache_value function
        prior to returning cached value if value_cached is FALSE.
        The Item_cache::get_cache function is overloaded to allow setting of the
        cache type.
     @ sql/item.h
        Bug#34384: Slow down on constant conversion.
        A flag named value_cached is added to the Item_cache class. It's set to TRUE
        when we need to start caching values when the store method is called.
        Function named cache_value() is added to the Item_cache class and derived classes.
     @ sql/item_cmpfunc.cc
        Bug#34384: Slow down on constant conversion.
        A helper function cache_converted_constant is added to the Arg_comparator class.
        It checks whether a given item can and should be cached and caches it if so.
        The Arg_comparator::set_cmp_func function now calls cache_converted_constant
        to cache constants if they need a type conversion.
     @ sql/item_cmpfunc.h
        Bug#34384: Slow down on constant conversion.
        The cache_converted_constant function is added to the Arg_comparator class.
        It checks whether a value can and should be cached and if so caches it.
     @ sql/item_subselect.cc
        Bug#34384: Slow down on constant conversion.
        Force immediate caching of subselect result.
     @ sql/item_xmlfunc.cc
        Bug#34384: Slow down on constant conversion.
     @ sql/sp_rcontext.cc
        Bug#34384: Slow down on constant conversion.
        Force immediate caching of values of an SP CASE function.

    modified:
      sql/item.cc
      sql/item.h
      sql/item_cmpfunc.cc
      sql/item_cmpfunc.h
      sql/item_subselect.cc
      sql/item_xmlfunc.cc
      sql/sp_rcontext.cc
=== modified file 'sql/item.cc'
--- a/sql/item.cc	2009-10-13 04:43:27 +0000
+++ b/sql/item.cc	2009-11-06 19:34:25 +0000
@@ -6958,7 +6958,22 @@ int stored_field_cmp_to_item(Field *fiel
 
 Item_cache* Item_cache::get_cache(const Item *item)
 {
-  switch (item->result_type()) {
+  return get_cache(item, item->result_type());
+}
+
+
+/**
+  Get a cache item of given type.
+
+  @param item         value to be cached
+  @param type         required type of cache
+
+  @return cache item
+*/
+
+Item_cache* Item_cache::get_cache(const Item *item, const Item_result type)
+{
+  switch (type) {
   case INT_RESULT:
     return new Item_cache_int();
   case REAL_RESULT:
@@ -6976,6 +6991,12 @@ Item_cache* Item_cache::get_cache(const 
   }
 }
 
+void Item_cache::store(Item *item)
+{
+  if (item)
+    example= item;
+  value_cached= FALSE;
+}
 
 void Item_cache::print(String *str, enum_query_type query_type)
 {
@@ -6987,17 +7008,19 @@ void Item_cache::print(String *str, enum
   str->append(')');
 }
 
-
-void Item_cache_int::store(Item *item)
+void  Item_cache_int::cache_value()
 {
-  value= item->val_int_result();
-  null_value= item->null_value;
-  unsigned_flag= item->unsigned_flag;
+  value_cached= TRUE;
+  value= example->val_int_result();
+  null_value= example->null_value;
+  unsigned_flag= example->unsigned_flag;
 }
 
 
 void Item_cache_int::store(Item *item, longlong val_arg)
 {
+  /* An explicit values is given, save it. */
+  value_cached= TRUE;
   value= val_arg;
   null_value= item->null_value;
   unsigned_flag= item->unsigned_flag;
@@ -7007,6 +7030,8 @@ void Item_cache_int::store(Item *item, l
 String *Item_cache_int::val_str(String *str)
 {
   DBUG_ASSERT(fixed == 1);
+  if (!value_cached)
+    cache_value();
   str->set(value, default_charset());
   return str;
 }
@@ -7015,21 +7040,49 @@ String *Item_cache_int::val_str(String *
 my_decimal *Item_cache_int::val_decimal(my_decimal *decimal_val)
 {
   DBUG_ASSERT(fixed == 1);
+  if (!value_cached)
+    cache_value();
   int2my_decimal(E_DEC_FATAL_ERROR, value, unsigned_flag, decimal_val);
   return decimal_val;
 }
 
+double Item_cache_int::val_real()
+{
+  DBUG_ASSERT(fixed == 1);
+  if (!value_cached)
+    cache_value();
+  return (double) value;
+}
 
-void Item_cache_real::store(Item *item)
+longlong Item_cache_int::val_int()
 {
-  value= item->val_result();
-  null_value= item->null_value;
+  DBUG_ASSERT(fixed == 1);
+  if (!value_cached)
+    cache_value();
+  return value;
 }
 
+void Item_cache_real::cache_value()
+{
+  value_cached= TRUE;
+  value= example->val_result();
+  null_value= example->null_value;
+}
+
+
+double Item_cache_real::val_real()
+{
+  DBUG_ASSERT(fixed == 1);
+  if (!value_cached)
+    cache_value();
+  return value;
+}
 
 longlong Item_cache_real::val_int()
 {
   DBUG_ASSERT(fixed == 1);
+  if (!value_cached)
+    cache_value();
   return (longlong) rint(value);
 }
 
@@ -7037,6 +7090,8 @@ longlong Item_cache_real::val_int()
 String* Item_cache_real::val_str(String *str)
 {
   DBUG_ASSERT(fixed == 1);
+  if (!value_cached)
+    cache_value();
   str->set_real(value, decimals, default_charset());
   return str;
 }
@@ -7045,15 +7100,18 @@ String* Item_cache_real::val_str(String 
 my_decimal *Item_cache_real::val_decimal(my_decimal *decimal_val)
 {
   DBUG_ASSERT(fixed == 1);
+  if (!value_cached)
+    cache_value();
   double2my_decimal(E_DEC_FATAL_ERROR, value, decimal_val);
   return decimal_val;
 }
 
 
-void Item_cache_decimal::store(Item *item)
+void Item_cache_decimal::cache_value()
 {
-  my_decimal *val= item->val_decimal_result(&decimal_value);
-  if (!(null_value= item->null_value) && val != &decimal_value)
+  value_cached= TRUE;
+  my_decimal *val= example->val_decimal_result(&decimal_value);
+  if (!(null_value= example->null_value) && val != &decimal_value)
     my_decimal2decimal(val, &decimal_value);
 }
 
@@ -7061,6 +7119,8 @@ double Item_cache_decimal::val_real()
 {
   DBUG_ASSERT(fixed);
   double res;
+  if (!value_cached)
+    cache_value();
   my_decimal2double(E_DEC_FATAL_ERROR, &decimal_value, &res);
   return res;
 }
@@ -7069,6 +7129,8 @@ longlong Item_cache_decimal::val_int()
 {
   DBUG_ASSERT(fixed);
   longlong res;
+  if (!value_cached)
+    cache_value();
   my_decimal2int(E_DEC_FATAL_ERROR, &decimal_value, unsigned_flag, &res);
   return res;
 }
@@ -7076,6 +7138,8 @@ longlong Item_cache_decimal::val_int()
 String* Item_cache_decimal::val_str(String *str)
 {
   DBUG_ASSERT(fixed);
+  if (!value_cached)
+    cache_value();
   my_decimal_round(E_DEC_FATAL_ERROR, &decimal_value, decimals, FALSE,
                    &decimal_value);
   my_decimal2string(E_DEC_FATAL_ERROR, &decimal_value, 0, 0, 0, str);
@@ -7085,15 +7149,18 @@ String* Item_cache_decimal::val_str(Stri
 my_decimal *Item_cache_decimal::val_decimal(my_decimal *val)
 {
   DBUG_ASSERT(fixed);
+  if (!value_cached)
+    cache_value();
   return &decimal_value;
 }
 
 
-void Item_cache_str::store(Item *item)
+void Item_cache_str::cache_value()
 {
-  value_buff.set(buffer, sizeof(buffer), item->collation.collation);
-  value= item->str_result(&value_buff);
-  if ((null_value= item->null_value))
+  value_cached= TRUE;
+  value_buff.set(buffer, sizeof(buffer), example->collation.collation);
+  value= example->str_result(&value_buff);
+  if ((null_value= example->null_value))
     value= 0;
   else if (value != &value_buff)
   {
@@ -7115,6 +7182,8 @@ double Item_cache_str::val_real()
   DBUG_ASSERT(fixed == 1);
   int err_not_used;
   char *end_not_used;
+  if (!value_cached)
+    cache_value();
   if (value)
     return my_strntod(value->charset(), (char*) value->ptr(),
 		      value->length(), &end_not_used, &err_not_used);
@@ -7126,6 +7195,8 @@ longlong Item_cache_str::val_int()
 {
   DBUG_ASSERT(fixed == 1);
   int err;
+  if (!value_cached)
+    cache_value();
   if (value)
     return my_strntoll(value->charset(), value->ptr(),
 		       value->length(), 10, (char**) 0, &err);
@@ -7133,9 +7204,21 @@ longlong Item_cache_str::val_int()
     return (longlong)0;
 }
 
+
+String* Item_cache_str::val_str(String *str)
+{
+  DBUG_ASSERT(fixed == 1);
+  if (!value_cached)
+    cache_value();
+  return value;
+}
+
+
 my_decimal *Item_cache_str::val_decimal(my_decimal *decimal_val)
 {
   DBUG_ASSERT(fixed == 1);
+  if (!value_cached)
+    cache_value();
   if (value)
     string2my_decimal(E_DEC_FATAL_ERROR, value, decimal_val);
   else
@@ -7146,6 +7229,8 @@ my_decimal *Item_cache_str::val_decimal(
 
 int Item_cache_str::save_in_field(Field *field, bool no_conversions)
 {
+  if (!value_cached)
+    cache_value();
   int res= Item_cache::save_in_field(field, no_conversions);
   return (is_varbinary && field->type() == MYSQL_TYPE_STRING &&
           value->length() < field->field_length) ? 1 : res;
@@ -7180,11 +7265,19 @@ bool Item_cache_row::setup(Item * item)
 
 void Item_cache_row::store(Item * item)
 {
+  for (uint i= 0; i < item_count; i++)
+    values[i]->store(item->element_index(i));
+}
+
+
+void Item_cache_row::cache_value()
+{
+  value_cached= TRUE;
   null_value= 0;
-  item->bring_value();
+  example->bring_value();
   for (uint i= 0; i < item_count; i++)
   {
-    values[i]->store(item->element_index(i));
+    values[i]->cache_value();
     null_value|= values[i]->null_value;
   }
 }

=== modified file 'sql/item.h'
--- a/sql/item.h	2009-08-28 10:55:59 +0000
+++ b/sql/item.h	2009-11-06 19:34:25 +0000
@@ -1025,7 +1025,11 @@ class sp_head;
 
 class Item_basic_constant :public Item
 {
+  table_map used_table_map;
 public:
+  Item_basic_constant(): Item(), used_table_map(0) {};
+  void set_used_tables(table_map map) { used_table_map= map; }
+  table_map used_tables() const { return used_table_map; }
   /* to prevent drop fixed flag (no need parent cleanup call) */
   void cleanup()
   {
@@ -2890,15 +2894,25 @@ protected:
   */  
   Field *cached_field;
   enum enum_field_types cached_field_type;
-public:
-  Item_cache(): 
-    example(0), used_table_map(0), cached_field(0), cached_field_type(MYSQL_TYPE_STRING) 
+  /*
+    TRUE <=> cache holds value of the last stored item (i.e actual value).
+    store() stores item to be cached and sets this flag to FALSE.
+    On the first call of val_xxx function if this flag is set to FALSE the 
+    cache_value() will be called to actually cache value of saved item.
+    cache_value() will set this flag to TRUE.
+  */
+  bool value_cached;
+public:
+  Item_cache():
+    example(0), used_table_map(0), cached_field(0), cached_field_type(MYSQL_TYPE_STRING),
+    value_cached(0)
   {
     fixed= 1; 
     null_value= 1;
   }
   Item_cache(enum_field_types field_type_arg):
-    example(0), used_table_map(0), cached_field(0), cached_field_type(field_type_arg)
+    example(0), used_table_map(0), cached_field(0), cached_field_type(field_type_arg),
+    value_cached(0)
   {
     fixed= 1;
     null_value= 1;
@@ -2918,10 +2932,10 @@ public:
       cached_field= ((Item_field *)item)->field;
     return 0;
   };
-  virtual void store(Item *)= 0;
   enum Type type() const { return CACHE_ITEM; }
   enum_field_types field_type() const { return cached_field_type; }
   static Item_cache* get_cache(const Item *item);
+  static Item_cache* get_cache(const Item* item, const Item_result type);
   table_map used_tables() const { return used_table_map; }
   virtual void keep_array() {}
   virtual void print(String *str, enum_query_type query_type);
@@ -2933,6 +2947,8 @@ public:
   {
     return this == item;
   }
+  virtual void store(Item *item);
+  virtual void cache_value()= 0;
 };
 
 
@@ -2941,18 +2957,19 @@ class Item_cache_int: public Item_cache
 protected:
   longlong value;
 public:
-  Item_cache_int(): Item_cache(), value(0) {}
+  Item_cache_int(): Item_cache(),
+    value(0) {}
   Item_cache_int(enum_field_types field_type_arg):
     Item_cache(field_type_arg), value(0) {}
 
-  void store(Item *item);
   void store(Item *item, longlong val_arg);
-  double val_real() { DBUG_ASSERT(fixed == 1); return (double) value; }
-  longlong val_int() { DBUG_ASSERT(fixed == 1); return value; }
+  double val_real();
+  longlong val_int();
   String* val_str(String *str);
   my_decimal *val_decimal(my_decimal *);
   enum Item_result result_type() const { return INT_RESULT; }
   bool result_as_longlong() { return TRUE; }
+  void cache_value();
 };
 
 
@@ -2960,14 +2977,15 @@ class Item_cache_real: public Item_cache
 {
   double value;
 public:
-  Item_cache_real(): Item_cache(), value(0) {}
+  Item_cache_real(): Item_cache(),
+    value(0) {}
 
-  void store(Item *item);
-  double val_real() { DBUG_ASSERT(fixed == 1); return value; }
+  double val_real();
   longlong val_int();
   String* val_str(String *str);
   my_decimal *val_decimal(my_decimal *);
   enum Item_result result_type() const { return REAL_RESULT; }
+  void cache_value();
 };
 
 
@@ -2978,12 +2996,12 @@ protected:
 public:
   Item_cache_decimal(): Item_cache() {}
 
-  void store(Item *item);
   double val_real();
   longlong val_int();
   String* val_str(String *str);
   my_decimal *val_decimal(my_decimal *);
   enum Item_result result_type() const { return DECIMAL_RESULT; }
+  void cache_value();
 };
 
 
@@ -3001,14 +3019,14 @@ public:
                    MYSQL_TYPE_VARCHAR &&
                  !((const Item_field *) item)->field->has_charset())
   {}
-  void store(Item *item);
   double val_real();
   longlong val_int();
-  String* val_str(String *) { DBUG_ASSERT(fixed == 1); return value; }
+  String* val_str(String *);
   my_decimal *val_decimal(my_decimal *);
   enum Item_result result_type() const { return STRING_RESULT; }
   CHARSET_INFO *charset() const { return value->charset(); };
   int save_in_field(Field *field, bool no_conversions);
+  void cache_value();
 };
 
 class Item_cache_row: public Item_cache
@@ -3018,7 +3036,8 @@ class Item_cache_row: public Item_cache
   bool save_array;
 public:
   Item_cache_row()
-    :Item_cache(), values(0), item_count(2), save_array(0) {}
+    :Item_cache(), values(0), item_count(2),
+    save_array(0) {}
   
   /*
     'allocate' used only in row transformer, to preallocate space for row 
@@ -3076,6 +3095,7 @@ public:
       values= 0;
     DBUG_VOID_RETURN;
   }
+  void cache_value();
 };
 
 

=== modified file 'sql/item_cmpfunc.cc'
--- a/sql/item_cmpfunc.cc	2009-10-05 05:27:36 +0000
+++ b/sql/item_cmpfunc.cc	2009-11-06 19:34:25 +0000
@@ -855,13 +855,13 @@ int Arg_comparator::set_cmp_func(Item_bo
 {
   enum enum_date_cmp_type cmp_type;
   ulonglong const_value= (ulonglong)-1;
+  thd= current_thd;
+  owner= owner_arg;
   a= a1;
   b= a2;
 
   if ((cmp_type= can_compare_as_dates(*a, *b, &const_value)))
   {
-    thd= current_thd;
-    owner= owner_arg;
     a_type= (*a)->field_type();
     b_type= (*b)->field_type();
     a_cache= 0;
@@ -869,6 +869,10 @@ int Arg_comparator::set_cmp_func(Item_bo
 
     if (const_value != (ulonglong)-1)
     {
+      /*
+        cache_converted_constant can't be used here because it can't
+        correctly convert a DATETIME value from string to int representation.
+      */
       Item_cache_int *cache= new Item_cache_int();
       /* Mark the cache as non-const to prevent re-caching. */
       cache->set_used_tables(1);
@@ -894,8 +898,6 @@ int Arg_comparator::set_cmp_func(Item_bo
            (*b)->field_type() == MYSQL_TYPE_TIME)
   {
     /* Compare TIME values as integers. */
-    thd= current_thd;
-    owner= owner_arg;
     a_cache= 0;
     b_cache= 0;
     is_nulls_eq= test(owner && owner->functype() == Item_func::EQUAL_FUNC);
@@ -914,10 +916,46 @@ int Arg_comparator::set_cmp_func(Item_bo
       return 1;
   }
 
+  a= cache_converted_constant(thd, a, &a_cache, type);
+  b= cache_converted_constant(thd, b, &b_cache, type);
   return set_compare_func(owner_arg, type);
 }
 
 
+/**
+  Convert and cache a constant.
+
+  @param value      [in]  An item to cache
+  @param cache_item [out] Placeholder for the cache item
+  @param type       [in]  Comparison type
+
+  @details
+    When given item is a constant and its type differs from comparison type
+    then cache its value to avoid type conversion of this constant on each
+    evaluation. In this case the value is cached and the reference to the cache
+    is returned.
+    Original value is returned otherwise.
+
+  @return cache item or original value.
+*/
+
+Item** Arg_comparator::cache_converted_constant(THD *thd, Item **value,
+                                                Item **cache_item,
+                                                Item_result type)
+{
+  /* Don't need cache if doing context analysis only. */
+  if (!thd->is_context_analysis_only() &&
+      (*value)->const_item() && type != (*value)->result_type())
+  {
+    Item_cache *cache= Item_cache::get_cache(*value, type);
+    cache->store(*value);
+    *cache_item= cache;
+    return cache_item;
+  }
+  return value;
+}
+
+
 void Arg_comparator::set_datetime_cmp_func(Item **a1, Item **b1)
 {
   thd= current_thd;
@@ -1556,6 +1594,7 @@ longlong Item_in_optimizer::val_int()
   bool tmp;
   DBUG_ASSERT(fixed == 1);
   cache->store(args[0]);
+  cache->cache_value();
   
   if (cache->null_value)
   {

=== modified file 'sql/item_cmpfunc.h'
--- a/sql/item_cmpfunc.h	2008-12-12 11:13:11 +0000
+++ b/sql/item_cmpfunc.h	2009-11-06 19:34:25 +0000
@@ -94,6 +94,8 @@ public:
                                                       ulonglong *const_val_arg);
 
   void set_datetime_cmp_func(Item **a1, Item **b1);
+  Item** cache_converted_constant(THD *thd, Item **value, Item **cache,
+                                  Item_result type);
   static arg_cmp_func comparator_matrix [5][2];
 
   friend class Item_func;

=== modified file 'sql/item_subselect.cc'
--- a/sql/item_subselect.cc	2009-09-04 08:14:54 +0000
+++ b/sql/item_subselect.cc	2009-11-06 19:34:25 +0000
@@ -475,6 +475,7 @@ Item_singlerow_subselect::select_transfo
 void Item_singlerow_subselect::store(uint i, Item *item)
 {
   row[i]->store(item);
+  row[i]->cache_value();
 }
 
 enum Item_result Item_singlerow_subselect::result_type() const
@@ -1826,6 +1827,7 @@ void subselect_engine::set_row(List<Item
     if (!(row[i]= Item_cache::get_cache(sel_item)))
       return;
     row[i]->setup(sel_item);
+    row[i]->store(sel_item);
   }
   if (item_list.elements > 1)
     res_type= ROW_RESULT;

=== modified file 'sql/item_xmlfunc.cc'
--- a/sql/item_xmlfunc.cc	2009-09-23 13:21:29 +0000
+++ b/sql/item_xmlfunc.cc	2009-11-06 19:34:25 +0000
@@ -941,14 +941,16 @@ static Item *create_comparator(MY_XPATH 
      in a loop through all of the nodes in the node set.
     */
 
-    Item *fake= new Item_string("", 0, xpath->cs);
+    Item_string *fake= new Item_string("", 0, xpath->cs);
+    /* Don't cache fake because its value will be changed during comparison.*/
+    fake->set_used_tables(RAND_TABLE_BIT);
     Item_nodeset_func *nodeset;
     Item *scalar, *comp;
     if (a->type() == Item::XPATH_NODESET)
     {
       nodeset= (Item_nodeset_func*) a;
       scalar= b;
-      comp= eq_func(oper, fake, scalar);
+      comp= eq_func(oper, (Item*)fake, scalar);
     }
     else
     {

=== modified file 'sql/sp_rcontext.cc'
--- a/sql/sp_rcontext.cc	2008-01-23 22:36:57 +0000
+++ b/sql/sp_rcontext.cc	2009-11-06 19:34:25 +0000
@@ -617,7 +617,7 @@ sp_rcontext::set_case_expr(THD *thd, int
   }
 
   m_case_expr_holders[case_expr_id]->store(case_expr_item);
-
+  m_case_expr_holders[case_expr_id]->cache_value();
   return FALSE;
 }
 


Attachment: [text/bzr-bundle] bzr/epotemkin@mysql.com-20091106193425-5e2hwmycjrxbqjop.bundle
Thread
bzr commit into mysql-5.1-bugteam branch (epotemkin:3200) Bug#34384Evgeny Potemkin6 Nov