#At file:///export/home/tmp/x/mysql-next-mr-bugfixing-49177/ based on revid:marc.alff@stripped
3309 Magne Mahre 2010-09-30
Bug#49177 MyISAM share list scalability problems
(Note: MyISAM issue only)
As the table cache size is increased, cache lookups and
(in particular) misses became increasingly more expensive.
The problem was caused by the cache lookup mechanism, which
was based on traversing a linked list, of <table cache size>
length, comparing the file names.
As the list was replaced by a hash table, the lookup
time dropped significantly when used on a large table cache.
The performance on smaller installations remains in the same
ballpark. Instead of growing exponentially, the lookup time
now grows more or less linearly, at least in the ballpark
with less than 100 000 open tables.
modified:
storage/myisam/ha_myisam.cc
storage/myisam/mi_close.c
storage/myisam/mi_dbug.c
storage/myisam/mi_keycache.c
storage/myisam/mi_open.c
storage/myisam/mi_panic.c
storage/myisam/mi_static.c
storage/myisam/myisamdef.h
=== modified file 'storage/myisam/ha_myisam.cc'
--- a/storage/myisam/ha_myisam.cc 2010-07-13 17:29:44 +0000
+++ b/storage/myisam/ha_myisam.cc 2010-09-30 09:29:51 +0000
@@ -642,7 +642,9 @@ ha_myisam::ha_myisam(handlerton *hton, T
HA_CAN_INSERT_DELAYED | HA_CAN_BIT_FIELD | HA_CAN_RTREEKEYS |
HA_HAS_RECORDS | HA_STATS_RECORDS_IS_EXACT),
can_enable_indexes(1)
-{}
+{
+ mi_init_table_cache(table_cache_size);
+}
handler *ha_myisam::clone(MEM_ROOT *mem_root)
{
=== modified file 'storage/myisam/mi_close.c'
--- a/storage/myisam/mi_close.c 2010-07-08 21:20:08 +0000
+++ b/storage/myisam/mi_close.c 2010-09-30 09:29:51 +0000
@@ -54,7 +54,9 @@ int mi_close(register MI_INFO *info)
info->opt_flag&= ~(READ_CACHE_USED | WRITE_CACHE_USED);
}
flag= !--share->reopen;
- myisam_open_list=list_delete(myisam_open_list,&info->open_list);
+
+ my_hash_delete(myisam_open_cache, (uchar *) info);
+
mysql_mutex_unlock(&share->intern_lock);
my_free(mi_get_rec_buff_ptr(info, info->rec_buff));
=== modified file 'storage/myisam/mi_dbug.c'
--- a/storage/myisam/mi_dbug.c 2010-06-07 12:05:34 +0000
+++ b/storage/myisam/mi_dbug.c 2010-09-30 09:29:51 +0000
@@ -181,14 +181,14 @@ void _mi_print_key(FILE *stream, registe
my_bool check_table_is_closed(const char *name, const char *where)
{
char filename[FN_REFLEN];
- LIST *pos;
+ uint idx;
DBUG_ENTER("check_table_is_closed");
(void) fn_format(filename,name,"",MI_NAME_IEXT,4+16+32);
mysql_mutex_lock(&THR_LOCK_myisam);
- for (pos=myisam_open_list ; pos ; pos=pos->next)
+ for (idx= 0; idx < myisam_open_cache->records; ++idx)
{
- MI_INFO *info=(MI_INFO*) pos->data;
+ MI_INFO *info=(MI_INFO*) my_hash_element(myisam_open_cache, idx);
MYISAM_SHARE *share=info->s;
if (!strcmp(share->unique_file_name,filename))
{
=== modified file 'storage/myisam/mi_keycache.c'
--- a/storage/myisam/mi_keycache.c 2009-12-05 01:26:15 +0000
+++ b/storage/myisam/mi_keycache.c 2010-09-30 09:29:51 +0000
@@ -137,16 +137,16 @@ int mi_assign_to_key_cache(MI_INFO *info
void mi_change_key_cache(KEY_CACHE *old_key_cache,
KEY_CACHE *new_key_cache)
{
- LIST *pos;
+ uint idx;
DBUG_ENTER("mi_change_key_cache");
/*
Lock list to ensure that no one can close the table while we manipulate it
*/
mysql_mutex_lock(&THR_LOCK_myisam);
- for (pos=myisam_open_list ; pos ; pos=pos->next)
+ for (idx= 0; idx < myisam_open_cache->records; ++idx)
{
- MI_INFO *info= (MI_INFO*) pos->data;
+ MI_INFO *info=(MI_INFO*) my_hash_element(myisam_open_cache, idx);
MYISAM_SHARE *share= info->s;
if (share->key_cache == old_key_cache)
mi_assign_to_key_cache(info, (ulonglong) ~0, new_key_cache);
=== modified file 'storage/myisam/mi_open.c'
--- a/storage/myisam/mi_open.c 2010-07-23 20:59:42 +0000
+++ b/storage/myisam/mi_open.c 2010-09-30 09:29:51 +0000
@@ -36,24 +36,78 @@ if (pos > end_pos) \
goto err; \
}
+/**
+ * Get the value used as hash key (helper function for the
+ * table cache hash). Function is used as a callback
+ * from the hash table
+ *
+ */
+static uchar *mi_table_cache_key(const uchar *record, size_t *length,
+ my_bool not_used __attribute__((unused)))
+{
+ MI_INFO *info= (MI_INFO *) record;
+ *length= strlen(info->s->unique_file_name);
+ return (uchar*) info->s->unique_file_name;
+}
-/******************************************************************************
-** Return the shared struct if the table is already open.
-** In MySQL the server will handle version issues.
-******************************************************************************/
+/**
+ * Initialize table cache hash
+ *
+ * @param cache_size Initial cache size (elements)
+ *
+ * Function is normally called from ha_myisam
+ * with the system variable table_cache_size used
+ * as cache_size.
+ */
+void mi_init_table_cache(ulong cache_size)
+{
+ if (!myisam_open_cache)
+ {
+ if (cache_size == 0)
+ cache_size= 32; /* default cache size */
+ my_hash_init(&myisam_open_hash, &my_charset_filename,
+ cache_size, 0, 0, mi_table_cache_key, 0, 0);
+ myisam_open_cache= &myisam_open_hash;
+ }
+}
+
+/**
+ * Retrieve the shared struct if the table is already
+ * open (i.e in the cache)
+ *
+ * @param filename table file name
+ * @return shared struct, 0 if not in the cache
+ *
+ */
MI_INFO *test_if_reopen(char *filename)
{
- LIST *pos;
+ HASH_SEARCH_STATE current_record;
+ int len= strlen(filename);
- for (pos=myisam_open_list ; pos ; pos=pos->next)
+ /* Initialize table cache on first call */
+ if (!myisam_open_cache)
+ mi_init_table_cache(0);
+
+ MI_INFO *info= (MI_INFO*) my_hash_first(myisam_open_cache,
+ (uchar *) filename,
+ len, ¤t_record);
+ /*
+ There might be more than one instance of a table share for
+ a given table in the cache. We're interested in the one with
+ last_version set, so we iterate until we find it
+ */
+ while (info)
{
- MI_INFO *info=(MI_INFO*) pos->data;
MYISAM_SHARE *share=info->s;
- if (!strcmp(share->unique_file_name,filename) && share->last_version)
- return info;
+ if (share->last_version)
+ break;
+
+ info= (MI_INFO*) my_hash_next(myisam_open_cache,
+ (uchar *) filename,
+ len, ¤t_record);
}
- return 0;
+ return info;
}
@@ -629,7 +683,8 @@ MI_INFO *mi_open(const char *name, int m
thr_lock_data_init(&share->lock,&m_info->lock,(void*) m_info);
#endif
m_info->open_list.data=(void*) m_info;
- myisam_open_list=list_add(myisam_open_list,&m_info->open_list);
+
+ my_hash_insert(myisam_open_cache, (uchar *) m_info);
mysql_mutex_unlock(&THR_LOCK_myisam);
=== modified file 'storage/myisam/mi_panic.c'
--- a/storage/myisam/mi_panic.c 2009-12-05 01:26:15 +0000
+++ b/storage/myisam/mi_panic.c 2010-09-30 09:29:51 +0000
@@ -26,54 +26,55 @@
int mi_panic(enum ha_panic_function flag)
{
int error=0;
- LIST *list_element,*next_open;
MI_INFO *info;
+ uint idx;
DBUG_ENTER("mi_panic");
mysql_mutex_lock(&THR_LOCK_myisam);
- for (list_element=myisam_open_list ; list_element ; list_element=next_open)
+ if (myisam_open_cache)
{
- next_open=list_element->next; /* Save if close */
- info=(MI_INFO*) list_element->data;
- switch (flag) {
- case HA_PANIC_CLOSE:
- mysql_mutex_unlock(&THR_LOCK_myisam); /* Not exactly right... */
- if (mi_close(info))
- error=my_errno;
- mysql_mutex_lock(&THR_LOCK_myisam);
- break;
- case HA_PANIC_WRITE: /* Do this to free databases */
+ for (idx= 0; idx < myisam_open_cache->records; ++idx)
+ {
+ info=(MI_INFO*) my_hash_element(myisam_open_cache, idx);
+ switch (flag) {
+ case HA_PANIC_CLOSE:
+ mysql_mutex_unlock(&THR_LOCK_myisam); /* Not exactly right... */
+ if (mi_close(info))
+ error=my_errno;
+ mysql_mutex_lock(&THR_LOCK_myisam);
+ break;
+ case HA_PANIC_WRITE: /* Do this to free databases */
#ifdef CANT_OPEN_FILES_TWICE
- if (info->s->options & HA_OPTION_READ_ONLY_DATA)
- break;
+ if (info->s->options & HA_OPTION_READ_ONLY_DATA)
+ break;
#endif
- if (flush_key_blocks(info->s->key_cache, info->s->kfile, FLUSH_RELEASE))
- error=my_errno;
- if (info->opt_flag & WRITE_CACHE_USED)
- if (flush_io_cache(&info->rec_cache))
- error=my_errno;
- if (info->opt_flag & READ_CACHE_USED)
- {
- if (flush_io_cache(&info->rec_cache))
- error=my_errno;
- reinit_io_cache(&info->rec_cache,READ_CACHE,0,
- (pbool) (info->lock_type != F_UNLCK),1);
- }
- if (info->lock_type != F_UNLCK && ! info->was_locked)
- {
- info->was_locked=info->lock_type;
- if (mi_lock_database(info,F_UNLCK))
- error=my_errno;
- }
+ if (flush_key_blocks(info->s->key_cache, info->s->kfile, FLUSH_RELEASE))
+ error=my_errno;
+ if (info->opt_flag & WRITE_CACHE_USED)
+ if (flush_io_cache(&info->rec_cache))
+ error=my_errno;
+ if (info->opt_flag & READ_CACHE_USED)
+ {
+ if (flush_io_cache(&info->rec_cache))
+ error=my_errno;
+ reinit_io_cache(&info->rec_cache,READ_CACHE,0,
+ (pbool) (info->lock_type != F_UNLCK),1);
+ }
+ if (info->lock_type != F_UNLCK && ! info->was_locked)
+ {
+ info->was_locked=info->lock_type;
+ if (mi_lock_database(info,F_UNLCK))
+ error=my_errno;
+ }
#ifdef CANT_OPEN_FILES_TWICE
- if (info->s->kfile >= 0 && mysql_file_close(info->s->kfile, MYF(0)))
- error = my_errno;
- if (info->dfile >= 0 && mysql_file_close(info->dfile, MYF(0)))
- error = my_errno;
- info->s->kfile=info->dfile= -1; /* Files aren't open anymore */
- break;
+ if (info->s->kfile >= 0 && mysql_file_close(info->s->kfile, MYF(0)))
+ error = my_errno;
+ if (info->dfile >= 0 && mysql_file_close(info->dfile, MYF(0)))
+ error = my_errno;
+ info->s->kfile=info->dfile= -1; /* Files aren't open anymore */
+ break;
#endif
- case HA_PANIC_READ: /* Restore to before WRITE */
+ case HA_PANIC_READ: /* Restore to before WRITE */
#ifdef CANT_OPEN_FILES_TWICE
{ /* Open closed files */
char name_buff[FN_REFLEN];
@@ -103,6 +104,7 @@ int mi_panic(enum ha_panic_function flag
info->was_locked=0;
}
break;
+ }
}
}
if (flag == HA_PANIC_CLOSE)
=== modified file 'storage/myisam/mi_static.c'
--- a/storage/myisam/mi_static.c 2010-08-05 12:34:19 +0000
+++ b/storage/myisam/mi_static.c 2010-09-30 09:29:51 +0000
@@ -22,7 +22,7 @@
#include "myisamdef.h"
#endif
-LIST *myisam_open_list=0;
+
uchar myisam_file_magic[]=
{ (uchar) 254, (uchar) 254,'\007', '\001', };
uchar myisam_pack_file_magic[]=
@@ -41,6 +41,14 @@ ulonglong myisam_max_temp_length= MAX_FI
ulong myisam_data_pointer_size=4;
ulonglong myisam_mmap_size= SIZE_T_MAX, myisam_mmap_used= 0;
+/*
+ MyISAM table cache. After initialization,
+ myisam_open_cache will point to myisam_open_hash
+*/
+HASH *myisam_open_cache=0;
+HASH myisam_open_hash;
+
+
static int always_valid(const char *filename __attribute__((unused)))
{
return 0;
=== modified file 'storage/myisam/myisamdef.h'
--- a/storage/myisam/myisamdef.h 2010-07-26 11:34:07 +0000
+++ b/storage/myisam/myisamdef.h 2010-09-30 09:29:51 +0000
@@ -25,6 +25,7 @@
#include <my_no_pthread.h>
#endif
#include <mysql/psi/mysql_file.h>
+#include "hash.h"
/* undef map from my_nosys; We need test-if-disk full */
#if defined(my_write)
@@ -477,7 +478,8 @@ extern mysql_mutex_t THR_LOCK_myisam;
/* Some extern variables */
-extern LIST *myisam_open_list;
+extern HASH *myisam_open_cache;
+extern HASH myisam_open_hash;
extern uchar myisam_file_magic[], myisam_pack_file_magic[];
extern uint myisam_read_vec[], myisam_readnext_vec[];
extern uint myisam_quick_table_bits;
@@ -759,6 +761,7 @@ my_bool mi_check_status(void* param);
void mi_disable_non_unique_index(MI_INFO *info, ha_rows rows);
extern MI_INFO *test_if_reopen(char *filename);
+extern void mi_init_table_cache(ulong size);
my_bool check_table_is_closed(const char *name, const char *where);
int mi_open_datafile(MI_INFO *info, MYISAM_SHARE *share, const char *orn_name,
File file_to_dup);
Attachment: [text/bzr-bundle] bzr/magne.mahre@sun.com-20100930092951-f4x0ddgfqe96kx67.bundle
