List:Commits« Previous MessageNext Message »
From:Evgeny Potemkin Date:September 19 2009 5:56pm
Subject:bzr commit into mysql-5.4 branch (epotemkin:2864) Bug#34384
View as plain text  
#At file:///work/bzrroot/34384-bug-next/ based on revid:epotemkin@stripped

 2864 Evgeny Potemkin	2009-09-19
      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.
      
      A test case isn't provided because all changes are internal and isn't visible
      outside.
      
      A flag named uncacheable is added to the Item class. It denies caching of the
      item.
      A flag named late_caching is added to the Item_cache class. It's set to TRUE
      when we need to postpone caching till the first use of cached value.
      A function named check_late_caching is added to the Item_cache flag. Its
      purpose is to check state of the late_caching flag and cache value accordingly.
      Item_cache_xxx::val_xxx functions are changed to call it prior to return
      cached value.
      The Arg_comparator::set_cmp_func function is changed to cache a value being
      compared if it's a constant and needs type conversion.
      Now the Item_func_like::fix_fields function denies caching arguments of the LIKE
      function.
      The Item_cache::get_cache function is overloaded to allow setting of the
      late_caching flag and cache type.
     @ mysql-test/r/range.result
        Adjusted a test case after fix for the bug#34384.
     @ sql/item.cc
        Bug#34384: Slow down on constant conversion.
        A flag named uncacheable is added to the Item class. It denies caching of the
        item.
        The Item_cache::get_cache function is overloaded to allow setting of the
        late_caching flag and cache type.
        Item_cache_xxx::val_xxx functions are changed to call it prior to return
        cached value.
     @ sql/item.h
        Bug#34384: Slow down on constant conversion.
        A function named check_late_caching is added to the Item_cache flag. Its
        purpose is to check state of the late_caching flag and cache value accordingly.
        The Item_cache::get_cache function is overloaded to allow setting of the
        late_caching flag and cache type.
     @ sql/item_cmpfunc.cc
        Bug#34384: Slow down on constant conversion.
        The Arg_comparator::set_cmp_func function is changed to cache a value being
        compared if it's a constant and needs type conversion.
     @ sql/item_cmpfunc.h
        Bug#34384: Slow down on constant conversion.
     @ sql/item_xmlfunc.cc
        Bug#34384: Slow down on constant conversion.

    modified:
      mysql-test/r/range.result
      sql/item.cc
      sql/item.h
      sql/item_cmpfunc.cc
      sql/item_cmpfunc.h
      sql/item_xmlfunc.cc
=== modified file 'mysql-test/r/range.result'
--- a/mysql-test/r/range.result	2008-06-10 22:27:52 +0000
+++ b/mysql-test/r/range.result	2009-09-19 17:55:58 +0000
@@ -1230,8 +1230,6 @@ c1
 Warnings:
 Warning	1366	Incorrect decimal value: 'A' for column 'c1' at row 1
 Warning	1292	Truncated incorrect DOUBLE value: 'A'
-Warning	1292	Truncated incorrect DOUBLE value: 'A'
-Warning	1292	Truncated incorrect DOUBLE value: 'A'
 DROP TABLE t1;
 create table t1 (a int,b int,key (b),key (a),key (b,a));
 insert into t1(a,b) values (1,2),(3,4),(5,6),(7,8);

=== modified file 'sql/item.cc'
--- a/sql/item.cc	2009-07-23 12:21:41 +0000
+++ b/sql/item.cc	2009-09-19 17:55:58 +0000
@@ -376,7 +376,8 @@ int Item::save_str_value_in_field(Field 
 Item::Item():
   is_expensive_cache(-1), rsize(0), name(0), orig_name(0), name_length(0),
   fixed(0), is_autogenerated_name(TRUE),
-  collation(&my_charset_bin, DERIVATION_COERCIBLE)
+  collation(&my_charset_bin, DERIVATION_COERCIBLE),
+  uncacheable(0)
 {
   marker= 0;
   maybe_null=null_value=with_sum_func=unsigned_flag=0;
@@ -7136,17 +7137,22 @@ bool field_is_equal_to_item(Field *field
 
 Item_cache* Item_cache::get_cache(const Item *item)
 {
-  switch (item->result_type()) {
+  return get_cache(item->result_type(), item, FALSE);
+}
+
+Item_cache* Item_cache::get_cache(const Item_result type, const Item *item, bool late_caching)
+{
+  switch (type) {
   case INT_RESULT:
-    return new Item_cache_int();
+    return new Item_cache_int(late_caching);
   case REAL_RESULT:
-    return new Item_cache_real();
+    return new Item_cache_real(late_caching);
   case DECIMAL_RESULT:
-    return new Item_cache_decimal();
+    return new Item_cache_decimal(late_caching);
   case STRING_RESULT:
-    return new Item_cache_str(item);
+    return new Item_cache_str(item,late_caching);
   case ROW_RESULT:
-    return new Item_cache_row();
+    return new Item_cache_row(late_caching);
   default:
     // should never be in real life
     DBUG_ASSERT(0);
@@ -7168,6 +7174,8 @@ void Item_cache::print(String *str, enum
 
 void Item_cache_int::store(Item *item)
 {
+  if (check_late_caching(item))
+    return;
   value= item->val_int_result();
   null_value= item->null_value;
   unsigned_flag= item->unsigned_flag;
@@ -7176,6 +7184,8 @@ void Item_cache_int::store(Item *item)
 
 void Item_cache_int::store(Item *item, longlong val_arg)
 {
+  if (check_late_caching(item))
+    return;
   value= val_arg;
   null_value= item->null_value;
   unsigned_flag= item->unsigned_flag;
@@ -7185,6 +7195,7 @@ void Item_cache_int::store(Item *item, l
 String *Item_cache_int::val_str(String *str)
 {
   DBUG_ASSERT(fixed == 1);
+  check_late_caching(NULL);
   str->set(value, default_charset());
   return str;
 }
@@ -7193,6 +7204,7 @@ String *Item_cache_int::val_str(String *
 my_decimal *Item_cache_int::val_decimal(my_decimal *decimal_val)
 {
   DBUG_ASSERT(fixed == 1);
+  check_late_caching(NULL);
   int2my_decimal(E_DEC_FATAL_ERROR, value, unsigned_flag, decimal_val);
   return decimal_val;
 }
@@ -7200,6 +7212,8 @@ my_decimal *Item_cache_int::val_decimal(
 
 void Item_cache_real::store(Item *item)
 {
+  if (check_late_caching(item))
+    return;
   value= item->val_result();
   null_value= item->null_value;
 }
@@ -7208,6 +7222,7 @@ void Item_cache_real::store(Item *item)
 longlong Item_cache_real::val_int()
 {
   DBUG_ASSERT(fixed == 1);
+  check_late_caching(NULL);
   return (longlong) rint(value);
 }
 
@@ -7215,6 +7230,7 @@ longlong Item_cache_real::val_int()
 String* Item_cache_real::val_str(String *str)
 {
   DBUG_ASSERT(fixed == 1);
+  check_late_caching(NULL);
   str->set_real(value, decimals, default_charset());
   return str;
 }
@@ -7223,6 +7239,7 @@ String* Item_cache_real::val_str(String 
 my_decimal *Item_cache_real::val_decimal(my_decimal *decimal_val)
 {
   DBUG_ASSERT(fixed == 1);
+  check_late_caching(NULL);
   double2my_decimal(E_DEC_FATAL_ERROR, value, decimal_val);
   return decimal_val;
 }
@@ -7230,6 +7247,8 @@ my_decimal *Item_cache_real::val_decimal
 
 void Item_cache_decimal::store(Item *item)
 {
+  if (check_late_caching(item))
+    return;
   my_decimal *val= item->val_decimal_result(&decimal_value);
   if (!(null_value= item->null_value) && val != &decimal_value)
     my_decimal2decimal(val, &decimal_value);
@@ -7239,6 +7258,7 @@ double Item_cache_decimal::val_real()
 {
   DBUG_ASSERT(fixed);
   double res;
+  check_late_caching(NULL);
   my_decimal2double(E_DEC_FATAL_ERROR, &decimal_value, &res);
   return res;
 }
@@ -7247,6 +7267,7 @@ longlong Item_cache_decimal::val_int()
 {
   DBUG_ASSERT(fixed);
   longlong res;
+  check_late_caching(NULL);
   my_decimal2int(E_DEC_FATAL_ERROR, &decimal_value, unsigned_flag, &res);
   return res;
 }
@@ -7254,6 +7275,7 @@ longlong Item_cache_decimal::val_int()
 String* Item_cache_decimal::val_str(String *str)
 {
   DBUG_ASSERT(fixed);
+  check_late_caching(NULL);
   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);
@@ -7263,12 +7285,15 @@ String* Item_cache_decimal::val_str(Stri
 my_decimal *Item_cache_decimal::val_decimal(my_decimal *val)
 {
   DBUG_ASSERT(fixed);
+  check_late_caching(NULL);
   return &decimal_value;
 }
 
 
 void Item_cache_str::store(Item *item)
 {
+  if (check_late_caching(item))
+    return;
   value_buff.set(buffer, sizeof(buffer), item->collation.collation);
   value= item->str_result(&value_buff);
   if ((null_value= item->null_value))
@@ -7293,6 +7318,7 @@ double Item_cache_str::val_real()
   DBUG_ASSERT(fixed == 1);
   int err_not_used;
   char *end_not_used;
+  check_late_caching(NULL);
   if (value)
     return my_strntod(value->charset(), (char*) value->ptr(),
 		      value->length(), &end_not_used, &err_not_used);
@@ -7304,6 +7330,7 @@ longlong Item_cache_str::val_int()
 {
   DBUG_ASSERT(fixed == 1);
   int err;
+  check_late_caching(NULL);
   if (value)
     return my_strntoll(value->charset(), value->ptr(),
 		       value->length(), 10, (char**) 0, &err);
@@ -7314,6 +7341,7 @@ longlong Item_cache_str::val_int()
 my_decimal *Item_cache_str::val_decimal(my_decimal *decimal_val)
 {
   DBUG_ASSERT(fixed == 1);
+  check_late_caching(NULL);
   if (value)
     string2my_decimal(E_DEC_FATAL_ERROR, value, decimal_val);
   else
@@ -7324,6 +7352,7 @@ my_decimal *Item_cache_str::val_decimal(
 
 int Item_cache_str::save_in_field(Field *field, bool no_conversions)
 {
+  check_late_caching(NULL);
   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;
@@ -7341,6 +7370,8 @@ bool Item_cache_row::allocate(uint num)
 
 bool Item_cache_row::setup(Item * item)
 {
+  if (check_late_caching(item))
+    return FALSE;
   example= item;
   if (!values && allocate(item->cols()))
     return 1;
@@ -7358,6 +7389,8 @@ bool Item_cache_row::setup(Item * item)
 
 void Item_cache_row::store(Item * item)
 {
+  if (check_late_caching(item))
+    return;
   null_value= 0;
   item->bring_value();
   for (uint i= 0; i < item_count; i++)

=== modified file 'sql/item.h'
--- a/sql/item.h	2009-06-24 17:00:59 +0000
+++ b/sql/item.h	2009-09-19 17:55:58 +0000
@@ -532,6 +532,7 @@ public:
                                            of its arguments is or contains a
                                            subselect. Computed by fix_fields. */
   Item_result cmp_context;              /* Comparison context */
+  bool uncacheable;
   // alloc & destruct is done as start of select using sql_alloc
   Item();
   /*
@@ -2995,14 +2996,24 @@ protected:
   Field *cached_field;
   enum enum_field_types cached_field_type;
 public:
+  bool late_caching;
   Item_cache(): 
-    example(0), used_table_map(0), cached_field(0), cached_field_type(MYSQL_TYPE_STRING) 
+    example(0), used_table_map(0), cached_field(0), cached_field_type(MYSQL_TYPE_STRING),
+    late_caching(0)
+  {
+    fixed= 1; 
+    null_value= 1;
+  }
+  Item_cache(bool late_caching_arg): 
+    example(0), used_table_map(0), cached_field(0), cached_field_type(MYSQL_TYPE_STRING),
+    late_caching(late_caching_arg)
   {
     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),
+    late_caching(0)
   {
     fixed= 1;
     null_value= 1;
@@ -3026,6 +3037,8 @@ public:
   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_result type, const Item* item,
+                                    bool late_caching);
   table_map used_tables() const { return used_table_map; }
   virtual void keep_array() {}
   virtual void print(String *str, enum_query_type query_type);
@@ -3037,6 +3050,18 @@ public:
   {
     return this == item;
   }
+  inline bool check_late_caching(Item *item)
+  {
+    if (!late_caching)
+      return FALSE;
+    if (item)
+      example= item;
+    else {
+      late_caching= FALSE;
+      store(example);
+    }
+    return TRUE;
+  }
 };
 
 
@@ -3046,13 +3071,24 @@ protected:
   longlong value;
 public:
   Item_cache_int(): Item_cache(), value(0) {}
+  Item_cache_int(bool late_caching_arg): Item_cache(late_caching_arg), 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()
+  {
+    DBUG_ASSERT(fixed == 1);
+    check_late_caching(NULL);
+    return (double) value;
+  }
+  longlong val_int()
+  {
+    DBUG_ASSERT(fixed == 1);
+    check_late_caching(NULL);
+    return value;
+  }
   String* val_str(String *str);
   my_decimal *val_decimal(my_decimal *);
   enum Item_result result_type() const { return INT_RESULT; }
@@ -3065,9 +3101,15 @@ class Item_cache_real: public Item_cache
   double value;
 public:
   Item_cache_real(): Item_cache(), value(0) {}
+  Item_cache_real(bool late_caching_arg): Item_cache(late_caching_arg), value(0) {}
 
   void store(Item *item);
-  double val_real() { DBUG_ASSERT(fixed == 1); return value; }
+  double val_real()
+  {
+    DBUG_ASSERT(fixed == 1);
+    check_late_caching(NULL);
+    return value;
+  }
   longlong val_int();
   String* val_str(String *str);
   my_decimal *val_decimal(my_decimal *);
@@ -3081,6 +3123,7 @@ protected:
   my_decimal decimal_value;
 public:
   Item_cache_decimal(): Item_cache() {}
+  Item_cache_decimal(bool late_caching_arg): Item_cache(late_caching_arg) {}
 
   void store(Item *item);
   double val_real();
@@ -3105,10 +3148,22 @@ public:
                    MYSQL_TYPE_VARCHAR &&
                  !((const Item_field *) item)->field->has_charset())
   {}
+  Item_cache_str(const Item *item, bool late_caching_arg) :
+    Item_cache(late_caching_arg), value(0),
+    is_varbinary(item->type() == FIELD_ITEM &&
+                 ((const Item_field *) item)->field->type() ==
+                   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 *)
+  {
+    DBUG_ASSERT(fixed == 1);
+    check_late_caching(NULL);
+    return value;
+  }
   my_decimal *val_decimal(my_decimal *);
   enum Item_result result_type() const { return STRING_RESULT; }
   CHARSET_INFO *charset() const { return value->charset(); };
@@ -3123,6 +3178,9 @@ class Item_cache_row: public Item_cache
 public:
   Item_cache_row()
     :Item_cache(), values(0), item_count(2), save_array(0) {}
+  Item_cache_row(bool late_caching_arg)
+    :Item_cache(late_caching_arg), values(0), item_count(2),
+    save_array(0) {}
   
   /*
     'allocate' used only in row transformer, to preallocate space for row 

=== modified file 'sql/item_cmpfunc.cc'
--- a/sql/item_cmpfunc.cc	2009-07-03 10:39:01 +0000
+++ b/sql/item_cmpfunc.cc	2009-09-19 17:55:58 +0000
@@ -524,6 +524,13 @@ void Item_bool_func2::fix_length_and_dec
   set_cmp_func();
 }
 
+void Item_bool_func2::set_cmp_func()
+{
+  /* Don't need cache for context analysis. */
+  if (!current_thd->is_context_analysis_only())
+    tmp_arg[0]->uncacheable= tmp_arg[1]->uncacheable= TRUE;
+  cmp.set_cmp_func(this, tmp_arg, tmp_arg+1);
+}
 
 int Arg_comparator::set_compare_func(Item_bool_func2 *item, Item_result type)
 {
@@ -847,6 +854,7 @@ int Arg_comparator::set_cmp_func(Item_bo
                                         Item_result type)
 {
   ulonglong const_value= (ulonglong)-1;
+  Item_cache *cache;
   a= a1;
   b= a2;
 
@@ -906,6 +914,28 @@ int Arg_comparator::set_cmp_func(Item_bo
       return 1;
   }
 
+  {
+    /*
+      When one of items being compared is a constant and its type
+      differs from comparison type then cache such item to avoid
+      type conversion of this constant on each evaluation.
+    */
+    if ((*a)->const_item() && !(*a)->uncacheable &&
+        type != (*a)->result_type())
+    {
+      cache= Item_cache::get_cache(type, *a, TRUE);
+      cache->store(*a);
+      a_cache= cache;
+      a= (Item **)&a_cache;
+    } else if ((*b)->const_item() && !(*b)->uncacheable &&
+               type != (*b)->result_type())
+    {
+      cache= Item_cache::get_cache(type, *b, TRUE);
+      cache->store(*b);
+      b_cache= cache;
+      b= (Item **)&b_cache;
+    }
+  }
   return set_compare_func(owner_arg, type);
 }
 
@@ -4465,6 +4495,11 @@ Item_func::optimize_type Item_func_like:
 bool Item_func_like::fix_fields(THD *thd, Item **ref)
 {
   DBUG_ASSERT(fixed == 0);
+
+  /* LIKE doesn't compare arguments directly, thus no reason to cache them. */
+  for (int i= 0; i < arg_count; i++)
+    args[i]->uncacheable= TRUE;
+
   if (Item_bool_func2::fix_fields(thd, ref) ||
       escape_item->fix_fields(thd, &escape_item))
     return TRUE;

=== modified file 'sql/item_cmpfunc.h'
--- a/sql/item_cmpfunc.h	2009-06-22 09:36:50 +0000
+++ b/sql/item_cmpfunc.h	2009-09-19 17:55:58 +0000
@@ -335,10 +335,7 @@ public:
   Item_bool_func2(Item *a,Item *b)
     :Item_int_func(a,b), cmp(tmp_arg, tmp_arg+1), abort_on_null(FALSE) {}
   void fix_length_and_dec();
-  void set_cmp_func()
-  {
-    cmp.set_cmp_func(this, tmp_arg, tmp_arg+1);
-  }
+  void set_cmp_func();
   optimize_type select_optimize() const { return OPTIMIZE_OP; }
   virtual enum Functype rev_functype() const { return UNKNOWN_FUNC; }
   bool have_rev_func() const { return rev_functype() != UNKNOWN_FUNC; }

=== modified file 'sql/item_xmlfunc.cc'
--- a/sql/item_xmlfunc.cc	2009-07-28 14:16:37 +0000
+++ b/sql/item_xmlfunc.cc	2009-09-19 17:55:58 +0000
@@ -952,6 +952,8 @@ static Item *create_comparator(MY_XPATH 
     */
 
     Item *fake= new Item_string("", 0, xpath->cs);
+    /* Don't cache fake because its value will be changed during comparison.*/
+    fake->uncacheable= TRUE;
     Item_nodeset_func *nodeset;
     Item *scalar, *comp;
     if (a->type() == Item::XPATH_NODESET)


Attachment: [text/bzr-bundle] bzr/epotemkin@mysql.com-20090919175558-eqekzibs1e9bjamh.bundle
Thread
bzr commit into mysql-5.4 branch (epotemkin:2864) Bug#34384Evgeny Potemkin19 Sep
  • Re: bzr commit into mysql-5.4 branch (epotemkin:2864) Bug#34384Øystein Grøvlen25 Sep
    • Re: bzr commit into mysql-5.4 branch (epotemkin:2864) Bug#34384Evgeny Potemkin25 Sep
      • Re: bzr commit into mysql-5.4 branch (epotemkin:2864) Bug#34384Øystein Grøvlen28 Sep
        • Re: bzr commit into mysql-5.4 branch (epotemkin:2864) Bug#34384Sergei Golubchik28 Sep
        • Re: bzr commit into mysql-5.4 branch (epotemkin:2864) Bug#34384Evgeny Potemkin29 Sep
  • Re: bzr commit into mysql-5.4 branch (epotemkin:2864) Bug#34384Øystein Grøvlen4 Oct