Re: [Maria-developers] [Commits] b2db8e8: MDEV-7949: Item_field::used_tables() takes 0.29% in OLTP RO
Hi Sanja, this patch looks Ok, just one minor suggestion inline. Though effect for given workload is quite moderate: Item_field::used_tables 0.21% -> 0.20% Item_basic_constant::used_tables 0.11% -> 0.11% Item_func::used_tables 0.05% -> 0.05% Item_sum::used_tables 0.00% -> 0.00% On Wed, May 13, 2015 at 04:17:26PM +0200, sanja@mariadb.com wrote:
revision-id: b2db8e85422a455a10cd28ef7e44977182528f70 parent(s): b22959903b89e798f8804ec9a815c88f75915cd9 committer: Oleksandr Byelkin branch nick: server timestamp: 2015-05-13 16:17:22 +0200 message:
MDEV-7949: Item_field::used_tables() takes 0.29% in OLTP RO
small sixes of used_tables() usage
...skip...
diff --git a/sql/item_func.h b/sql/item_func.h index 0d57c2b..c48cd84 100644 --- a/sql/item_func.h +++ b/sql/item_func.h @@ -1337,7 +1337,7 @@ class Item_func_sleep :public Item_int_func const char *func_name() const { return "sleep"; } table_map used_tables() const { - return Item_int_func::used_tables() | RAND_TABLE_BIT; + return used_tables_cache | RAND_TABLE_BIT; } bool is_expensive() { return 1; } longlong val_int(); @@ -1591,7 +1591,7 @@ class Item_func_get_lock :public Item_int_func void fix_length_and_dec() { max_length=1; maybe_null=1;} table_map used_tables() const { - return Item_int_func::used_tables() | RAND_TABLE_BIT; + return used_tables_cache | RAND_TABLE_BIT; } bool const_item() const { return 0; } bool is_expensive() { return 1; } @@ -1611,7 +1611,7 @@ class Item_func_release_lock :public Item_int_func void fix_length_and_dec() { max_length= 1; maybe_null= 1;} table_map used_tables() const { - return Item_int_func::used_tables() | RAND_TABLE_BIT; + return used_tables_cache | RAND_TABLE_BIT; } bool const_item() const { return 0; } bool is_expensive() { return 1; } @@ -1724,7 +1724,7 @@ class Item_func_set_user_var :public Item_func void fix_length_and_dec(); table_map used_tables() const { - return Item_func::used_tables() | RAND_TABLE_BIT; + return used_tables_cache | RAND_TABLE_BIT; } bool const_item() const { return 0; } bool is_expensive() { return 1; } Was there any reason for this change? Compiler should be clever enough to inline used_tables() of base class here.
Regards, Sergey
Hi Sanja,
this patch looks Ok, just one minor suggestion inline. Though effect for given workload is quite moderate:
Item_field::used_tables 0.21% -> 0.20% Item_basic_constant::used_tables 0.11% -> 0.11% Item_func::used_tables 0.05% -> 0.05% Item_sum::used_tables 0.00% -> 0.00% Virtual function can't be inlined (AFAIK). That was original request, but it require too much changes for 10.1 I just fixed obvious thing to make less calls. IMHO this is the only
On 19.05.15 14:00, Sergey Vojtovich wrote: thing we can do in 10.1 taking into account soon RC.
On Wed, May 13, 2015 at 04:17:26PM +0200, sanja@mariadb.com wrote:
revision-id: b2db8e85422a455a10cd28ef7e44977182528f70 parent(s): b22959903b89e798f8804ec9a815c88f75915cd9 committer: Oleksandr Byelkin branch nick: server timestamp: 2015-05-13 16:17:22 +0200 message:
MDEV-7949: Item_field::used_tables() takes 0.29% in OLTP RO
small sixes of used_tables() usage
...skip...
diff --git a/sql/item_func.h b/sql/item_func.h index 0d57c2b..c48cd84 100644 --- a/sql/item_func.h +++ b/sql/item_func.h @@ -1337,7 +1337,7 @@ class Item_func_sleep :public Item_int_func const char *func_name() const { return "sleep"; } table_map used_tables() const { - return Item_int_func::used_tables() | RAND_TABLE_BIT; + return used_tables_cache | RAND_TABLE_BIT; } bool is_expensive() { return 1; } longlong val_int(); @@ -1591,7 +1591,7 @@ class Item_func_get_lock :public Item_int_func void fix_length_and_dec() { max_length=1; maybe_null=1;} table_map used_tables() const { - return Item_int_func::used_tables() | RAND_TABLE_BIT; + return used_tables_cache | RAND_TABLE_BIT; } bool const_item() const { return 0; } bool is_expensive() { return 1; } @@ -1611,7 +1611,7 @@ class Item_func_release_lock :public Item_int_func void fix_length_and_dec() { max_length= 1; maybe_null= 1;} table_map used_tables() const { - return Item_int_func::used_tables() | RAND_TABLE_BIT; + return used_tables_cache | RAND_TABLE_BIT; } bool const_item() const { return 0; } bool is_expensive() { return 1; } @@ -1724,7 +1724,7 @@ class Item_func_set_user_var :public Item_func void fix_length_and_dec(); table_map used_tables() const { - return Item_func::used_tables() | RAND_TABLE_BIT; + return used_tables_cache | RAND_TABLE_BIT; } bool const_item() const { return 0; } bool is_expensive() { return 1; } Was there any reason for this change? Compiler should be clever enough to inline used_tables() of base class here. I am not sure that it is _so_ clever with virtual functions even if knows basic type because of possible inheritance and re-implementinmg (also can be checked by compiler, but again I am not sure)
Regards, Sergey _______________________________________________ commits mailing list commits@mariadb.org https://lists.askmonty.org/cgi-bin/mailman/listinfo/commits
Virtual function can't be inlined (AFAIK). That was original request, but it require too much changes for 10.1
I am not sure that it is _so_ clever with virtual functions even if knows basic type because of possible inheritance and re-implementinmg (also can be checked by compiler, but again I am not sure)
Recent GCC versions have a devirtualization pass, followed by inlining: -fdevirtualize Attempt to convert calls to virtual functions to direct calls. This is done both within a procedure and interprocedurally as part of indirect inlining (-findirect-inlining) and interprocedural constant propagation (-fipa-cp). Enabled at levels -O2, -O3, -Os. I don't know whether it helps here or not. -- Laurynas
participants (3)
-
Laurynas Biveinis -
Oleksandr Byelkin -
Sergey Vojtovich