List:Commits« Previous MessageNext Message »
From:Magne Mahre Date:September 30 2010 9:29am
Subject:bzr commit into mysql-next-mr-bugfixing branch (magne.mahre:3309) Bug#49177
View as plain text  
#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, &current_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, &current_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
Thread
bzr commit into mysql-next-mr-bugfixing branch (magne.mahre:3309) Bug#49177Magne Mahre30 Sep
  • Re: bzr commit into mysql-next-mr-bugfixing branch (magne.mahre:3309)Bug#49177Konstantin Osipov7 Oct
    • Re: bzr commit into mysql-next-mr-bugfixing branch(magne.mahre:3309) Bug#49177Sergey Vojtovich7 Oct
    • Re: bzr commit into mysql-next-mr-bugfixing branch (magne.mahre:3309)Bug#49177Magne Mæhre21 Oct
  • Re: bzr commit into mysql-next-mr-bugfixing branch(magne.mahre:3309) Bug#49177Sergey Vojtovich8 Oct
    • Re: bzr commit into mysql-next-mr-bugfixing branch (magne.mahre:3309)Bug#49177Magne Mæhre21 Oct