3896 Dmitry Lenev 2012-05-15
WL#5772 "Add partitioned Table Definition Cache to avoid
using LOCK_open and its derivatives in DML queries".
Review change #8:
- Improved comments.
- Renamed Table_cache::rename_table_all() to free_table_all().
- Changed size of TABLE_SHARE::cache_element array from
MAX_TABLE_CACHES to table_cache_instances.
modified:
sql/sql_base.cc
sql/sql_base.h
sql/sql_const.h
sql/sys_vars.cc
sql/table.cc
sql/table.h
3895 Dmitry Lenev 2012-05-15
WL#5772 "Add partitioned Table Definition Cache to avoid
using LOCK_open and its derivatives in DML queries".
Review change #7:
- Adjust name of some methods to better reflect what they do.
- Make former static functions which are critical for performance
inline methods.
modified:
sql/sql_base.cc
sql/sql_base.h
sql/sql_test.cc
sql/table.cc
=== modified file 'sql/sql_base.cc'
--- a/sql/sql_base.cc 2012-05-15 15:10:59 +0000
+++ b/sql/sql_base.cc 2012-05-15 19:36:12 +0000
@@ -782,7 +782,13 @@ uint cached_table_definitions(void)
void Table_cache::free_unused_tables_if_necessary(THD *thd)
{
- /* We have too many TABLE instances around let us try to get rid of them. */
+ /*
+ We have too many TABLE instances around let us try to get rid of them.
+
+ Note that we might need to free more than one TABLE object, and thus
+ need the below loop, in case when table_cache_size is changed dynamically,
+ at server run time.
+ */
if (m_table_count > table_cache_size_per_instance && m_unused_tables)
{
mysql_mutex_lock(&LOCK_open);
@@ -9693,22 +9699,20 @@ void tdc_flush_unused_tables()
/**
- Remove all or some (depending on parameter) TABLE objects for
- the table from all table cache instances.
+ Remove and free all or some (depending on parameter) TABLE objects
+ for the table from all table cache instances.
@param thd Thread context
@param remove_type Type of removal. @sa tdc_remove_table().
@param key Key identifying table.
@param key_length Length of key.
- TODO: Find better name? free_something() ?
-
@note Caller should own LOCK_open and locks on all table cache
instances.
*/
-void Table_cache::remove_table_all(THD *thd,
- enum_tdc_remove_table_type remove_type,
- const char *key, uint key_length)
+void Table_cache::free_table_all(THD *thd,
+ enum_tdc_remove_table_type remove_type,
+ const char *key, uint key_length)
{
Table_cache::assert_owner_all_and_tdc();
@@ -9812,7 +9816,7 @@ void tdc_remove_table(THD *thd, enum_tdc
used.
*/
share->version= 0;
- Table_cache::remove_table_all(thd, remove_type, key, key_length);
+ Table_cache::free_table_all(thd, remove_type, key, key_length);
}
else
(void) my_hash_delete(&table_def_cache, (uchar*) share);
=== modified file 'sql/sql_base.h'
--- a/sql/sql_base.h 2012-05-15 15:10:59 +0000
+++ b/sql/sql_base.h 2012-05-15 19:36:12 +0000
@@ -655,6 +655,11 @@ struct Table_cache_element
class Table_cache
{
+public:
+
+ /** Maximum supported number of table cache instances. */
+ static const int MAX_TABLE_CACHES= 64;
+
private:
/**
The table cache lock protects the following data:
@@ -663,10 +668,11 @@ private:
2) m_cache hash.
3) used_tables, free_tables lists in Table_cache_element objects in
this cache.
- 4) the element in TABLE_SHARE::cache_element[] array that corresponds
+ 4) m_table_count - total number of TABLE objects in this cache.
+ 5) the element in TABLE_SHARE::cache_element[] array that corresponds
to this cache,
- 5) in_use member in TABLE object.
- 6) Also ownership of mutexes for all caches are required to update
+ 6) in_use member in TABLE object.
+ 7) Also ownership of mutexes for all caches are required to update
the refresh_version and table_def_shutdown_in_progress variables
and TABLE_SHARE::version member.
@@ -758,9 +764,9 @@ public:
inline bool add_used_table(THD *thd, TABLE *table);
inline void remove_table(TABLE *table);
- static void remove_table_all(THD *thd,
- enum_tdc_remove_table_type remove_type,
- const char *key, uint key_length);
+ static void free_table_all(THD *thd,
+ enum_tdc_remove_table_type remove_type,
+ const char *key, uint key_length);
static void free_all_unused_tables();
=== modified file 'sql/sql_const.h'
--- a/sql/sql_const.h 2012-05-11 16:05:27 +0000
+++ b/sql/sql_const.h 2012-05-15 19:36:12 +0000
@@ -127,8 +127,6 @@
cache can contain at least all tables of a given statement.
*/
#define TABLE_DEF_CACHE_MIN 400
-/** Maximum supported number of table cache instances. */
-#define MAX_TABLE_CACHES 64
/*
Stack reservation.
=== modified file 'sql/sys_vars.cc'
--- a/sql/sys_vars.cc 2012-05-14 20:05:23 +0000
+++ b/sql/sys_vars.cc 2012-05-15 19:36:12 +0000
@@ -2735,7 +2735,7 @@ static Sys_var_ulong Sys_table_cache_siz
static Sys_var_ulong Sys_table_cache_instances(
"table_open_cache_instances", "The number of table cache instances",
READ_ONLY GLOBAL_VAR(table_cache_instances), CMD_LINE(REQUIRED_ARG),
- VALID_RANGE(1, MAX_TABLE_CACHES), DEFAULT(1),
+ VALID_RANGE(1, Table_cache::MAX_TABLE_CACHES), DEFAULT(1),
BLOCK_SIZE(1));
static Sys_var_ulong Sys_thread_cache_size(
=== modified file 'sql/table.cc'
--- a/sql/table.cc 2012-05-15 15:10:59 +0000
+++ b/sql/table.cc 2012-05-15 19:36:12 +0000
@@ -322,6 +322,7 @@ TABLE_SHARE *alloc_table_share(TABLE_LIS
char *key_buff, *path_buff;
char path[FN_REFLEN];
uint path_length;
+ Table_cache_element **cache_element_array;
DBUG_ENTER("alloc_table_share");
DBUG_PRINT("enter", ("table: '%s'.'%s'",
table_list->db, table_list->table_name));
@@ -334,6 +335,8 @@ TABLE_SHARE *alloc_table_share(TABLE_LIS
&share, sizeof(*share),
&key_buff, key_length,
&path_buff, path_length + 1,
+ &cache_element_array,
+ table_cache_instances * sizeof(*cache_element_array),
NULL))
{
memset(share, 0, sizeof(*share));
@@ -360,6 +363,10 @@ TABLE_SHARE *alloc_table_share(TABLE_LIS
share->m_flush_tickets.empty();
+ memset(cache_element_array, 0,
+ table_cache_instances * sizeof(*cache_element_array));
+ share->cache_element= cache_element_array;
+
memcpy((char*) &share->mem_root, (char*) &mem_root, sizeof(mem_root));
mysql_mutex_init(key_TABLE_SHARE_LOCK_ha_data,
&share->LOCK_ha_data, MY_MUTEX_INIT_FAST);
@@ -3221,8 +3228,6 @@ uint Wait_for_flush::get_deadlock_weight
@retval TRUE A deadlock is found. A victim is remembered
by the visitor.
@retval FALSE No deadlocks, it's OK to begin wait.
-
- QQ: Should we move this method to sql_base.cc to allow inlining?
*/
bool TABLE_SHARE::visit_subgraph(Wait_for_flush *wait_for_flush,
=== modified file 'sql/table.h'
--- a/sql/table.h 2012-05-11 16:05:27 +0000
+++ b/sql/table.h 2012-05-15 19:36:12 +0000
@@ -585,12 +585,14 @@ struct TABLE_SHARE
mysql_mutex_t LOCK_ha_data; /* To protect access to ha_data */
TABLE_SHARE *next, **prev; /* Link to unused shares */
/**
- Elements of table cache respresenting this table in each of Table_cache
- instances. Each element of the array is protected by Table_cache::m_lock
- in the corresponding Table_cache. False sharing should not be a problem
- in this case as elements of this array are supposed to be updated rarely.
+ Array of table_cache_instances pointers to elements of table caches
+ respresenting this table in each of Table_cache instances.
+ Allocated along with the share itself in alloc_table_share().
+ Each element of the array is protected by Table_cache::m_lock in the
+ corresponding Table_cache. False sharing should not be a problem in
+ this case as elements of this array are supposed to be updated rarely.
*/
- Table_cache_element *cache_element[MAX_TABLE_CACHES];
+ Table_cache_element **cache_element;
/* The following is copied to each TABLE on OPEN */
Field **field;
No bundle (reason: useless for push emails).
| Thread |
|---|
| • bzr push into mysql-trunk branch (Dmitry.Lenev:3895 to 3896) WL#5772 | Dmitry Lenev | 16 May |
