List:Commits« Previous MessageNext Message »
From:dlenev Date:March 25 2008 11:08am
Subject:bk commit into 6.0 tree (dlenev:1.2563) WL#3726
View as plain text  
Below is the list of changes that have just been committed into a local
6.0 repository of dlenev.  When dlenev does a push these changes
will be propagated to the main repository and, within 24 hours after the
push, to the public repository.
For information on how to access the public repository
see http://dev.mysql.com/doc/mysql/en/installing-source-tree.html

ChangeSet@stripped, 2008-03-25 14:08:30+03:00, dlenev@stripped +6 -0
  WL#3726 "DDL locking for all metadata objects"
  
  Work in progress.
  
  Replaced hand-coded lists in metadata locking subsystem with lists
  based on newly introduced doubly linked intrusive parametrized list
  template (I_P_List).
  
  This simplifies code implementing meta-data locking but makes API a
  bit less consistent (ATM API is more C-oriented rather than C++-oriented).

  sql/meta_lock.cc@stripped, 2008-03-25 14:08:22+03:00, dlenev@stripped +110 -132
    Replaced hand-coded lists in MDL_LOCK and MDL_CONTEXT with I_P_List.

  sql/meta_lock.h@stripped, 2008-03-25 14:08:22+03:00, dlenev@stripped +70 -37
    Replaced hand-coded lists in MDL_LOCK and MDL_CONTEXT with I_P_List.
    Replaced mdl_get_first_lock()/mdl_get_next_lock() pair with one function
    mdl_get_locks() returning I_P_List_iterator.

  sql/sql_base.cc@stripped, 2008-03-25 14:08:22+03:00, dlenev@stripped +2 -1
    mdl_get_first_lock()/mdl_get_next_lock() pair was replaced with one
    function mdl_get_locks() returning I_P_List_iterator.

  sql/sql_class.cc@stripped, 2008-03-25 14:08:23+03:00, dlenev@stripped +41 -8
    Since MDL_CONTEXT is no longer can be copied around we can no longer do
    trivial copy of the whole Open_tables_state when backing it up and
    resetting or restoring from backup. Instead we have to resort to member
    by member copying/resetting and using special routines for MDL_CONTEXT. 

  sql/sql_class.h@stripped, 2008-03-25 14:08:23+03:00, dlenev@stripped +0 -15
    Since MDL_CONTEXT is no longer can be copied around we can no longer do
    trivial copy of the whole Open_tables_state when backing it up and
    resetting or restoring from backup. Instead we have to resort to member
    by member copying/resetting and using special routines for MDL_CONTEXT. 

  sql/sql_list.h@stripped, 2008-03-25 14:08:23+03:00, dlenev@stripped +100 -0
    Added I_P_List template class for parametrized intrusive doubly linked
    lists and I_P_List_iterator for corresponding iterator. Unlike for I_List<>
    list elements of such list can participate in several lists. Unlike List<>
    such lists are dobly-linked and intrusive.

diff -Nrup a/sql/meta_lock.cc b/sql/meta_lock.cc
--- a/sql/meta_lock.cc	2008-03-06 15:26:55 +03:00
+++ b/sql/meta_lock.cc	2008-03-25 14:08:22 +03:00
@@ -30,24 +30,30 @@ struct MDL_CONTEXT;
 
 struct MDL_LOCK
 {
-  MDL_EL *active_readers;
+  I_P_List<MDL_EL, MDL_EL_lock> active_readers;
   /*
     There can be several upgraders and active writers
     belonging to the same context.
   */
-  MDL_EL *active_readers_waiting_upgrade;
-  MDL_EL *active_writers;
-  MDL_EL *waiting_writers;
+  I_P_List<MDL_EL, MDL_EL_lock> active_readers_waiting_upgrade;
+  I_P_List<MDL_EL, MDL_EL_lock> active_writers;
+  I_P_List<MDL_EL, MDL_EL_lock> waiting_writers;
   uint   users;
   void   *cached_object;
   mdl_cached_object_release_hook cached_object_release_hook;
+
+  MDL_LOCK() : cached_object(0), cached_object_release_hook(0) {}
+
   MDL_EL *get_key_owner()
   {
-     return active_readers ? active_readers : (active_readers_waiting_upgrade ?
-                                               active_readers_waiting_upgrade :
-                                               (active_writers ? active_writers :
-                                                                 waiting_writers));
+     return !active_readers.is_empty() ?
+            active_readers.head() :
+            (!active_readers_waiting_upgrade.is_empty() ?
+             active_readers_waiting_upgrade.head() :
+             (!active_writers.is_empty() ?
+              active_writers.head() : waiting_writers.head()));
   }
+
   bool has_no_other_users()
   {
     return (users == 1);
@@ -116,14 +122,20 @@ void mdl_destroy()
    Initialize a metadata locking context.
 
    This is to be called when a new server connection is created.
-   This is a no-cost operation to just reset to NULL the list
-   of lock requests that this originator currently has and
-   links context to the THD object for this connection.
+
+   @note This routine assumes that it gets properly MDL_CONTEXT
+         constructed object as input and links context to the THD
+         object for this connection.
+
+   QQ: May be we should simply accept the fact that MDL_CONTEXT
+       is a C++ class and get rid of this routine, or even simply
+       use more C++ style for the meta-data locking subsystem as
+       whole?
 */
 
 void mdl_context_init(MDL_CONTEXT *context, THD *thd)
 {
-  context->locks= 0;
+  DBUG_ASSERT(context->locks.is_empty());
   context->thd= thd;
 }
 
@@ -142,7 +154,7 @@ void mdl_context_init(MDL_CONTEXT *conte
 
 void mdl_context_destroy(MDL_CONTEXT *context)
 {
-  DBUG_ASSERT(context->locks == 0);
+  DBUG_ASSERT(context->locks.is_empty());
 }
 
 
@@ -154,17 +166,12 @@ void mdl_context_destroy(MDL_CONTEXT *co
    needs to open table for new HANDLER independently of already
    open HANDLERs and add this table/metadata lock to the set of
    tables open/metadata locks for HANDLERs afterwards.
-
-   QQ: Does it makes sense to hide the fact that we
-       simply copy MDL_CONTEXT contents given that
-       in Open_tables_state::set_open_tables_state()
-       copies context directly anyway.
 */
 
 void mdl_context_backup_and_reset(MDL_CONTEXT *ctx, MDL_CONTEXT *backup)
 {
-  *backup= *ctx;
-  ctx->locks= 0;
+  backup->locks.empty();
+  ctx->locks.swap(backup->locks);
 }
 
 
@@ -174,8 +181,8 @@ void mdl_context_backup_and_reset(MDL_CO
 
 void mdl_context_restore(MDL_CONTEXT *ctx, MDL_CONTEXT *backup)
 {
-  DBUG_ASSERT(ctx->locks == 0);
-  *ctx= *backup;
+  DBUG_ASSERT(ctx->locks.is_empty());
+  ctx->locks.swap(backup->locks);
 }
 
 
@@ -189,16 +196,16 @@ void mdl_context_merge(MDL_CONTEXT *dst,
 
   DBUG_ASSERT(dst->thd == src->thd);
 
-  if (src->locks)
+  if (!src->locks.is_empty())
   {
-    for (l= src->locks; l; l= l->next_context)
+    I_P_List_iterator<MDL_EL, MDL_EL_context> it(src->locks);
+    while ((l= it++))
     {
       DBUG_ASSERT(l->ctx);
       l->ctx= dst;
+      dst->locks.push_front(l);
     }
-    src->locks->next_context= dst->locks;
-    dst->locks= src->locks;
-    src->locks= 0;
+    src->locks.empty();
   }
 }
 
@@ -324,9 +331,7 @@ void mdl_add_lock(MDL_CONTEXT *context, 
     lock->type= SHARED_PENDING;
   DBUG_ASSERT(!lock->ctx);
   lock->ctx= context;
-
-  lock->next_context= context->locks;
-  context->locks= lock;
+  context->locks.push_front(lock);
   DBUG_VOID_RETURN;
 }
 
@@ -353,10 +358,12 @@ void mdl_add_lock(MDL_CONTEXT *context, 
 void mdl_free_locks(MDL_CONTEXT *context)
 {
 #ifndef DBUG_OFF
-  for (MDL_EL *l= context->locks; l; l= l->next_context)
+  MDL_EL *l;
+  I_P_List_iterator<MDL_EL, MDL_EL_context> it(context->locks);
+  while ((l= it++))
     l->ctx= 0;
 #endif
-  context->locks= 0;
+  context->locks.empty();
 }
 
 
@@ -369,13 +376,13 @@ void mdl_free_locks(MDL_CONTEXT *context
 
 static MDL_LOCK* get_lock_object(void)
 {
-  return (MDL_LOCK *)my_malloc(sizeof(MDL_LOCK), MYF(MY_ZEROFILL));
+  return new MDL_LOCK();
 }
 
 
 static void release_lock_object(MDL_LOCK *lock)
 {
-  my_free(lock, MYF(0));
+  delete lock;
 }
 
 
@@ -415,8 +422,7 @@ bool mdl_acquire_shared_lock(MDL_EL *l)
   if (!(lock= (MDL_LOCK *)hash_search(&mdl_locks, (uchar*)l->key, l->key_length)))
   {
     lock= get_lock_object();
-    lock->active_readers= l;
-    l->next_lock= 0;
+    lock->active_readers.push_front(l);
     lock->users= 1;
     my_hash_insert(&mdl_locks, (uchar*)lock);
     l->type= SHARED;
@@ -424,9 +430,10 @@ bool mdl_acquire_shared_lock(MDL_EL *l)
   }
   else
   {
-    if (!(lock->active_writers || lock->waiting_writers ||
-          lock->active_readers_waiting_upgrade) ||
-        (lock->active_writers && lock->active_writers->ctx == l->ctx))
+    if ((lock->active_writers.is_empty() && lock->waiting_writers.is_empty() &&
+         lock->active_readers_waiting_upgrade.is_empty()) ||
+        (!lock->active_writers.is_empty() &&
+         lock->active_writers.head()->ctx == l->ctx))
     {
       /*
         When writers come from the same context we can satisfy our shared
@@ -435,8 +442,7 @@ bool mdl_acquire_shared_lock(MDL_EL *l)
         QQ: May be it is better always return error in this specific case
             (as later we always discover that there is an error) ?
       */
-      l->next_lock= lock->active_readers;
-      lock->active_readers= l;
+      lock->active_readers.push_front(l);
       lock->users++;
       l->type= SHARED;
       l->lock= lock;
@@ -464,10 +470,11 @@ bool mdl_acquire_shared_lock(MDL_EL *l)
 
 void mdl_acquire_exclusive_locks(MDL_CONTEXT *context)
 {
-  MDL_EL *l, *lh, **lp;
+  MDL_EL *l, *lh;
   MDL_LOCK *lock;
   bool signalled= FALSE;
   const char *proc_info;
+  I_P_List_iterator<MDL_EL, MDL_EL_context> it(context->locks);
 
   DBUG_ASSERT(context->thd == current_thd);
   /*
@@ -481,22 +488,20 @@ void mdl_acquire_exclusive_locks(MDL_CON
   safe_mutex_assert_not_owner(&LOCK_open);
 
   pthread_mutex_lock(&LOCK_mdl);
-  for (l= context->locks; l; l= l->next_context)
+  while ((l= it++))
   {
     DBUG_ASSERT(l->type == EXCLUSIVE_PENDING);
     if (!(lock= (MDL_LOCK *)hash_search(&mdl_locks, (uchar*)l->key, l->key_length)))
     {
       lock= get_lock_object();
-      lock->waiting_writers= l;
-      l->next_lock= 0;
+      lock->waiting_writers.push_front(l);
       lock->users= 1;
       my_hash_insert(&mdl_locks, (uchar*)lock);
       l->lock= lock;
     }
     else
     {
-      l->next_lock= lock->waiting_writers;
-      lock->waiting_writers= l;
+      lock->waiting_writers.push_front(l);
       lock->users++;
       l->lock= lock;
     }
@@ -504,17 +509,19 @@ void mdl_acquire_exclusive_locks(MDL_CON
 
   while (1)
   {
-    for (l= context->locks; l; l= l->next_context)
+    it.rewind();
+    while ((l= it++))
     {
       lock= l->lock;
 
-      if ((lh= lock->active_readers))
+      if ((lh= lock->active_readers.head()))
       {
         signalled= notify_thread_having_shared_lock(context->thd,
                                                     lh->ctx->thd);
         break;
       }
-      else if (lock->active_writers || lock->active_readers_waiting_upgrade)
+      else if (!lock->active_writers.is_empty() ||
+               !lock->active_readers_waiting_upgrade.is_empty())
       {
         /*
           Exclusive MDL owner won't wait on table-level lock the same
@@ -542,17 +549,12 @@ void mdl_acquire_exclusive_locks(MDL_CON
       pthread_cond_timedwait(&COND_mdl, &LOCK_mdl, &abstime);
     }
   }
-  for (l= context->locks; l; l= l->next_context)
+  it.rewind();
+  while ((l= it++))
   {
     lock= l->lock;
-    /* remove lock from waiters list */
-    lp= &(lock->waiting_writers);
-    while (*lp != l)
-      lp= &((*lp)->next_lock);
-    *lp= (*lp)->next_lock;
-    /* set it as and exclusive */
-    l->next_lock= lock->active_writers;
-    lock->active_writers= l;
+    lock->waiting_writers.remove(l);
+    lock->active_writers.push_front(l);
     l->type= EXCLUSIVE;
     if (lock->cached_object)
       (*lock->cached_object_release_hook)(l->lock->cached_object);
@@ -581,8 +583,9 @@ void mdl_upgrade_shared_lock_to_exclusiv
   char key[MAX_DBKEY_LENGTH];
   uint key_length;
   bool signalled= FALSE;
-  MDL_EL *l, *lh, **lp;
+  MDL_EL *l, *lh;
   MDL_LOCK *lock;
+  I_P_List_iterator<MDL_EL, MDL_EL_context> it(context->locks);
 
   DBUG_ENTER("mdl_upgrade_shared_lock_to_exclusive");
   DBUG_PRINT("enter", ("db=%s name=%s", db, name));
@@ -595,43 +598,35 @@ void mdl_upgrade_shared_lock_to_exclusiv
   safe_mutex_assert_not_owner(&LOCK_open);
 
   pthread_mutex_lock(&LOCK_mdl);
-  for (l= context->locks; l; l= l->next_context)
+  while ((l= it++))
     if (l->key_length == key_length && !memcmp(l->key, key, key_length) &&
         l->type == SHARED)
     {
       DBUG_PRINT("info", ("found shared lock for upgrade"));
       l->type= SHARED_PENDING_UPGRADE;
       lock= l->lock;
-      /* exclude lock from readers list */
-      lp= &(lock->active_readers);
-      while (*lp != l)
-      {
-        DBUG_PRINT("info", ("*lp=%p l=%p", *lp, l));
-        lp= &((*lp)->next_lock);
-      }
-      *lp= (*lp)->next_lock;
-      /* include lock in upgraders list */
-      l->next_lock= lock->active_readers_waiting_upgrade;
-      lock->active_readers_waiting_upgrade= l;
+      lock->active_readers.remove(l);
+      lock->active_readers_waiting_upgrade.push_front(l);
     }
 
   while (1)
   {
     DBUG_PRINT("info", ("looking at conflicting locks"));
-    for (l= context->locks; l; l= l->next_context)
+    it.rewind();
+    while ((l= it++))
     {
       if (l->type == SHARED_PENDING_UPGRADE)
       {
         lock= l->lock;
 
-        if ((lh= lock->active_readers))
+        if ((lh= lock->active_readers.head()))
         {
           DBUG_PRINT("info", ("found active readers"));
           signalled= notify_thread_having_shared_lock(context->thd,
                                                       lh->ctx->thd);
           break;
         }
-        else if (lock->active_writers)
+        else if (!lock->active_writers.is_empty())
         {
           DBUG_PRINT("info", ("found active writers"));
           signalled= TRUE;
@@ -658,18 +653,13 @@ void mdl_upgrade_shared_lock_to_exclusiv
     }
   }
 
-  for (l= context->locks; l; l= l->next_context)
+  it.rewind();
+  while ((l= it++))
     if (l->type == SHARED_PENDING_UPGRADE)
     {
       lock= l->lock;
-      /* remove lock from waiters list */
-      lp= &(lock->active_readers_waiting_upgrade);
-      while (*lp != l)
-        lp= &((*lp)->next_lock);
-      *lp= (*lp)->next_lock;
-      /* set it as exclusive */
-      l->next_lock= lock->active_writers;
-      lock->active_writers= l;
+      lock->active_readers_waiting_upgrade.remove(l);
+      lock->active_writers.push_front(l);
       l->type= EXCLUSIVE;
       if (lock->cached_object)
         (*lock->cached_object_release_hook)(l->lock->cached_object);
@@ -715,11 +705,10 @@ bool mdl_try_acquire_exclusive_lock(MDL_
   if (!(lock= (MDL_LOCK *)hash_search(&mdl_locks, (uchar*)l->key, l->key_length)))
   {
     lock= get_lock_object();
-    lock->active_writers= l;
+    lock->active_writers.push_front(l);
     lock->users= 1;
     my_hash_insert(&mdl_locks, (uchar*)lock);
     l->type= EXCLUSIVE;
-    l->next_lock= 0;
     l->lock= lock;
     lock= 0;
   }
@@ -732,12 +721,7 @@ bool mdl_try_acquire_exclusive_lock(MDL_
            presence in the context lists and MDL_EL::lock values.
   */
   if (lock)
-  {
-    MDL_EL **lp;
-    for (lp= &context->locks; *lp != l; lp= &((*lp)->next_context))
-      continue;
-    *lp= l->next_context;
-  }
+    context->locks.remove(l);
 
   return lock;
 }
@@ -760,6 +744,7 @@ void mdl_wait_for_locks(MDL_CONTEXT *con
   MDL_EL *l;
   MDL_LOCK *lock;
   const char *proc_info;
+  I_P_List_iterator<MDL_EL, MDL_EL_context> it(context->locks);
 
   safe_mutex_assert_not_owner(&LOCK_open);
   DBUG_ASSERT(context->thd == current_thd);
@@ -785,12 +770,14 @@ void mdl_wait_for_locks(MDL_CONTEXT *con
     */
     mysql_ha_flush(context->thd);
     pthread_mutex_lock(&LOCK_mdl);
-    for (l= context->locks; l; l= l->next_context)
+    it.rewind();
+    while ((l= it++))
     {
       DBUG_ASSERT(l->type == SHARED_PENDING);
       if ((lock= (MDL_LOCK *)hash_search(&mdl_locks, (uchar*)l->key, l->key_length)) &&
-          (lock->active_writers || lock->active_readers_waiting_upgrade ||
-           lock->waiting_writers))
+          !(lock->active_writers.is_empty() &&
+            lock->active_readers_waiting_upgrade.is_empty() &&
+            lock->waiting_writers.is_empty()))
         break;
     }
     if (!l)
@@ -812,7 +799,6 @@ void mdl_wait_for_locks(MDL_CONTEXT *con
 
 static void release_lock(MDL_EL *l)
 {
-  MDL_EL **lp;
   MDL_LOCK *lock;
 
   DBUG_ENTER("release_lock");
@@ -834,21 +820,18 @@ static void release_lock(MDL_EL *l)
     switch (l->type)
     {
       case SHARED:
-        lp= &(lock->active_readers);
+        lock->active_readers.remove(l);
         break;
       case EXCLUSIVE_PENDING:
-        lp= &(lock->waiting_writers);
+        lock->waiting_writers.remove(l);
         break;
       case EXCLUSIVE:
-        lp= &(lock->active_writers);
+        lock->active_writers.remove(l);
         break;
       default:
         /* TODO Really? How about problems during lock upgrade ? */
         DBUG_ASSERT(0);
     }
-    while (*lp != l)
-      lp= &((*lp)->next_lock);
-    *lp= (*lp)->next_lock;
     lock->users--;
   }
 
@@ -871,12 +854,13 @@ static void release_lock(MDL_EL *l)
 void mdl_release_locks(MDL_CONTEXT *context)
 {
   MDL_EL *l;
+  I_P_List_iterator<MDL_EL, MDL_EL_context> it(context->locks);
   DBUG_ENTER("mdl_release_locks");
 
   safe_mutex_assert_not_owner(&LOCK_open);
 
   pthread_mutex_lock(&LOCK_mdl);
-  for (l= context->locks; l; l= l->next_context)
+  while ((l= it++))
   {
     DBUG_PRINT("info", ("found lock to release l=%p", l));
     /*
@@ -910,24 +894,23 @@ void mdl_release_locks(MDL_CONTEXT *cont
 
 void mdl_release_exclusive_locks(MDL_CONTEXT *context)
 {
-  MDL_EL **l;
+  MDL_EL *l;
+  I_P_List_iterator<MDL_EL, MDL_EL_context> it(context->locks);
 
   safe_mutex_assert_not_owner(&LOCK_open);
 
   pthread_mutex_lock(&LOCK_mdl);
-  for (l= &context->locks; *l; )
+  while ((l= it++))
   {
-    if ((*l)->type == EXCLUSIVE)
+    if (l->type == EXCLUSIVE)
     {
-      release_lock(*l);
+      release_lock(l);
 #ifndef DBUG_OFF
-      (*l)->ctx= 0;
-      (*l)->lock= 0;
+      l->ctx= 0;
+      l->lock= 0;
 #endif
-      *l= (*l)->next_context;
+      context->locks.remove(l);
     }
-    else
-      l= &((*l)->next_context);
   }
   pthread_cond_broadcast(&COND_mdl);
   pthread_mutex_unlock(&LOCK_mdl);
@@ -944,19 +927,15 @@ void mdl_release_exclusive_locks(MDL_CON
 
 void mdl_release_lock(MDL_CONTEXT *context, MDL_EL *lr)
 {
-  MDL_EL **l;
-
   safe_mutex_assert_not_owner(&LOCK_open);
 
   pthread_mutex_lock(&LOCK_mdl);
-  for (l= &context->locks; *l != lr; l= &((*l)->next_context))
-    continue;
-  release_lock(*l);
+  release_lock(lr);
 #ifndef DBUG_OFF
-  (*l)->ctx= 0;
-  (*l)->lock= 0;
+  lr->ctx= 0;
+  lr->lock= 0;
 #endif
-  *l= (*l)->next_context;
+  context->locks.remove(lr);
   pthread_cond_broadcast(&COND_mdl);
   pthread_mutex_unlock(&LOCK_mdl);
 }
@@ -971,23 +950,20 @@ void mdl_release_lock(MDL_CONTEXT *conte
 
 void mdl_downgrade_exclusive_locks(MDL_CONTEXT *context)
 {
-  MDL_EL *l, **lp;
+  MDL_EL *l;
   MDL_LOCK *lock;
+  I_P_List_iterator<MDL_EL, MDL_EL_context> it(context->locks);
 
   safe_mutex_assert_not_owner(&LOCK_open);
 
   pthread_mutex_lock(&LOCK_mdl);
-  for (l= context->locks; l; l= l->next_context)
+  while ((l= it++))
     if (l->type == EXCLUSIVE)
     {
       lock= l->lock;
-      lp= &(lock->active_writers);
-      while (*lp != l)
-        lp= &((*lp)->next_lock);
-      *lp= (*lp)->next_lock;
+      lock->active_writers.remove(l);
       l->type= SHARED;
-      l->next_lock= lock->active_readers;
-      lock->active_readers= l;
+      lock->active_readers.push_front(l);
     }
   pthread_cond_broadcast(&COND_mdl);
   pthread_mutex_unlock(&LOCK_mdl);
@@ -1013,10 +989,12 @@ bool mdl_is_exclusive_lock_owner(MDL_CON
   char key[MAX_DBKEY_LENGTH];
   uint key_length;
   MDL_EL *l;
+  I_P_List_iterator<MDL_EL, MDL_EL_context> it(context->locks);
+
   int4store(key, type);
   key_length= (uint) (strmov(strmov(key+4, db)+1, name)-key)+1;
-  for (l= context->locks; l && (l->key_length != key_length || memcmp(l->key, key, key_length));
-       l= l->next_context)
+
+  while ((l= it++) && (l->key_length != key_length || memcmp(l->key, key, key_length)))
     continue;
   return (l && l->type == EXCLUSIVE);
 }
@@ -1034,8 +1012,8 @@ bool mdl_is_exclusive_lock_owner(MDL_CON
 bool mdl_has_pending_conflicting_lock(MDL_EL *l)
 {
   /* QQ: looks like we don't need any locking here ? */
-  return (l->lock->waiting_writers ||
-          l->lock->active_readers_waiting_upgrade);
+  return !(l->lock->waiting_writers.is_empty() &&
+           l->lock->active_readers_waiting_upgrade.is_empty());
 }
 
 
diff -Nrup a/sql/meta_lock.h b/sql/meta_lock.h
--- a/sql/meta_lock.h	2008-03-06 15:26:55 +03:00
+++ b/sql/meta_lock.h	2008-03-25 14:08:22 +03:00
@@ -17,17 +17,7 @@
 
 struct MDL_EL;
 struct MDL_LOCK;
-
-/**
-   Context of the owner of metadata locks. I.e. each server
-   connection has such a context.
-*/
-
-struct MDL_CONTEXT
-{
-  MDL_EL   *locks;
-  THD      *thd;
-};
+struct MDL_CONTEXT;
 
 
 /*
@@ -46,9 +36,6 @@ enum enum_mdl_type {SHARED_PENDING=0, SH
    the key consists of <0 (=table)>+<database name>+<table name>.
    Later in this document this triple will be referred to simply as
    "key" or "name".
-
-   TODO: By using double linked lists instead of single linked lists
-         we can make MDL code more simple and efficient.
 */
 
 struct MDL_EL
@@ -56,10 +43,24 @@ struct MDL_EL
   char        *key;
   uint        key_length;
   enum        enum_mdl_type type;
-  /* Next element in the list of lock requests for this context. */
+
+private:
+  /**
+     Pointers for participating in the list of lock requests for this context.
+  */
   MDL_EL      *next_context;
-  /* Next element in the list of satisfied/pending requests for the lock. */
+  MDL_EL      **prev_context;
+  /**
+     Pointers for participating in the list of satisfied/pending requests
+     for the lock.
+  */
   MDL_EL      *next_lock;
+  MDL_EL      **prev_lock;
+
+  friend struct MDL_EL_context;
+  friend struct MDL_EL_lock;
+
+public:
   /*
     Pointer to the lock object for this lock request. Valid only if this lock
     request is satisified or is present in the list of pending lock requests
@@ -70,6 +71,54 @@ struct MDL_EL
 };
 
 
+/**
+   Helper class which specifies which members of MDL_EL are used for
+   participation in the list lock requests belonging to one context.
+*/
+
+struct MDL_EL_context
+{
+  static inline MDL_EL **next_ptr(MDL_EL *l)
+  {
+    return &l->next_context;
+  }
+  static inline MDL_EL ***prev_ptr(MDL_EL *l)
+  {
+    return &l->prev_context;
+  }
+};
+
+
+/**
+   Helper class which specifies which members of MDL_EL are used for
+   participation in the list of satisfied/pending requests for the lock.
+*/
+
+struct MDL_EL_lock
+{
+  static inline MDL_EL **next_ptr(MDL_EL *l)
+  {
+    return &l->next_lock;
+  }
+  static inline MDL_EL ***prev_ptr(MDL_EL *l)
+  {
+    return &l->prev_lock;
+  }
+};
+
+
+/**
+   Context of the owner of metadata locks. I.e. each server
+   connection has such a context.
+*/
+
+struct MDL_CONTEXT
+{
+  I_P_List <MDL_EL, MDL_EL_context> locks;
+  THD      *thd;
+};
+
+
 void mdl_init();
 void mdl_destroy();
 
@@ -106,35 +155,19 @@ bool mdl_has_pending_conflicting_lock(MD
 
 inline bool mdl_has_non_freed_locks(MDL_CONTEXT *context)
 {
-  return context->locks;
-}
-
-
-/**
-   Get first lock request in the context (i.e. get initial
-   value for iterating over all lock requests in the context).
-
-   TODO: Try to get rid of mdl_get_first_lock()/mdl_get_next_lock()
-         by implementing new intrusive list and iterator templates
-         (current I_List implementation does not allow its elements
-         to participate in several intrusive lists).
-*/
-
-inline MDL_EL *mdl_get_first_lock(MDL_CONTEXT *ctx)
-{
-  return ctx->locks;
+  return !context->locks.is_empty();
 }
 
 
 /**
-   Given the current lock request get next lock request in the context.
+   Get iterator for walkin through all lock requests in the context.
 */
 
-inline MDL_EL *mdl_get_next_lock(MDL_CONTEXT *ctx, MDL_EL *current)
+inline I_P_List_iterator<MDL_EL, MDL_EL_context> mdl_get_locks(MDL_CONTEXT *ctx)
 {
-  return current->next_context;
+  I_P_List_iterator<MDL_EL, MDL_EL_context> result(ctx->locks);
+  return result;
 }
-
 
 /**
    Give metadata lock request object for the table get table definition
diff -Nrup a/sql/sql_base.cc b/sql/sql_base.cc
--- a/sql/sql_base.cc	2008-03-06 21:40:07 +03:00
+++ b/sql/sql_base.cc	2008-03-25 14:08:22 +03:00
@@ -8629,7 +8629,8 @@ static void tdc_wait_for_old_versions(TH
     */
     mysql_ha_flush(thd);
     pthread_mutex_lock(&LOCK_open);
-    for (l= mdl_get_first_lock(context); l; l= mdl_get_next_lock(context, l))
+    I_P_List_iterator<MDL_EL, MDL_EL_context> it= mdl_get_locks(context);
+    while ((l= it++))
     {
       mdl_get_tdc_key(l, &key);
       if ((share= (TABLE_SHARE*) hash_search(&table_def_cache, (uchar*) key.str,
diff -Nrup a/sql/sql_class.cc b/sql/sql_class.cc
--- a/sql/sql_class.cc	2008-03-06 15:26:56 +03:00
+++ b/sql/sql_class.cc	2008-03-25 14:08:23 +03:00
@@ -198,9 +198,12 @@ bool foreign_key_prefix(Key *a, Key *b)
 ****************************************************************************/
 
 Open_tables_state::Open_tables_state(THD *thd, ulong version_arg)
-  :version(version_arg), state_flags(0U)
+  : open_tables(0), temporary_tables(0), handler_tables(0), derived_tables(0),
+    lock(0), locked_tables(0), extra_lock(0), prelocked_mode(NON_PRELOCKED),
+    version(version_arg), state_flags(0U)
 {
-  reset_open_tables_state(thd);
+  mdl_context_init(&mdl_context, thd);
+  mdl_context_init(&handler_mdl_context, thd);
 }
 
 /*
@@ -2766,9 +2769,29 @@ Security_context::restore_security_conte
 void THD::reset_n_backup_open_tables_state(Open_tables_state *backup)
 {
   DBUG_ENTER("reset_n_backup_open_tables_state");
-  backup->set_open_tables_state(this);
-  reset_open_tables_state(this);
-  state_flags|= Open_tables_state::BACKUPS_AVAIL;
+  /*
+    QQ: Is there any better way to handle this without allowing to
+        copy I_P_List<> ? May be by introducing swap() method for
+        Open_tables_state and MDL_CONTEXT ?
+  */
+  backup->open_tables= open_tables;
+  backup->temporary_tables= temporary_tables;
+  backup->handler_tables= handler_tables;
+  backup->derived_tables= derived_tables;
+  backup->lock= lock;
+  backup->locked_tables= locked_tables;
+  backup->extra_lock= extra_lock;
+  backup->prelocked_mode= prelocked_mode;
+  backup->version= version;
+  backup->current_tablenr= current_tablenr;
+  backup->state_flags= state_flags;
+  open_tables= temporary_tables= handler_tables= derived_tables= 0;
+  extra_lock= lock= locked_tables= 0;
+  prelocked_mode= NON_PRELOCKED;
+  state_flags= Open_tables_state::BACKUPS_AVAIL;
+  mdl_context_backup_and_reset(&mdl_context, &backup->mdl_context);
+  mdl_context_backup_and_reset(&handler_mdl_context,
+                               &backup->handler_mdl_context);
   DBUG_VOID_RETURN;
 }
 
@@ -2784,9 +2807,19 @@ void THD::restore_backup_open_tables_sta
               handler_tables == 0 && derived_tables == 0 &&
               lock == 0 && locked_tables == 0 &&
               prelocked_mode == NON_PRELOCKED);
-  mdl_context_destroy(&mdl_context);
-  mdl_context_destroy(&handler_mdl_context);
-  set_open_tables_state(backup);
+  open_tables= backup->open_tables;
+  temporary_tables= backup->temporary_tables;
+  handler_tables= backup->handler_tables;
+  derived_tables= backup->derived_tables;
+  lock= backup->lock;
+  locked_tables= backup->locked_tables;
+  extra_lock= backup->extra_lock;
+  prelocked_mode= backup->prelocked_mode;
+  version= backup->version;
+  current_tablenr= backup->current_tablenr;
+  state_flags= backup->state_flags;
+  mdl_context_restore(&mdl_context, &backup->mdl_context);
+  mdl_context_restore(&handler_mdl_context, &backup->handler_mdl_context);
   DBUG_VOID_RETURN;
 }
 
diff -Nrup a/sql/sql_class.h b/sql/sql_class.h
--- a/sql/sql_class.h	2008-03-06 15:26:56 +03:00
+++ b/sql/sql_class.h	2008-03-25 14:08:23 +03:00
@@ -912,21 +912,6 @@ public:
   Open_tables_state() : state_flags(0U) { }
 
   Open_tables_state(THD *thd, ulong version_arg);
-
-  void set_open_tables_state(Open_tables_state *state)
-  {
-    *this= *state;
-  }
-
-  void reset_open_tables_state(THD *thd)
-  {
-    open_tables= temporary_tables= handler_tables= derived_tables= 0;
-    extra_lock= lock= locked_tables= 0;
-    prelocked_mode= NON_PRELOCKED;
-    state_flags= 0U;
-    mdl_context_init(&mdl_context, thd);
-    mdl_context_init(&handler_mdl_context, thd);
-  }
 };
 
 /**
diff -Nrup a/sql/sql_list.h b/sql/sql_list.h
--- a/sql/sql_list.h	2007-06-01 11:43:54 +04:00
+++ b/sql/sql_list.h	2008-03-25 14:08:23 +03:00
@@ -580,6 +580,106 @@ public:
   inline T* operator++(int) { return (T*) base_ilist_iterator::next(); }
 };
 
+
+template <typename T, typename B> class I_P_List_iterator;
+
+
+/**
+   Intrusive parameterized list.
+
+   Unlike I_List does not require its elements to be descendant of ilink
+   class and therefore allows them to participate in several such lists
+   simultaneously.
+
+   Unlike List is doubly-linked list and thus supports efficient deletion
+   of element without iterator.
+
+   @param T  Type of elements which will belong to list.
+   @param B  Class which via its methods specifies which members
+             of T should be used for participating in this list.
+             Here is typical layout of such class:
+
+             struct B
+             {
+               static inline T **next_ptr(T *el)
+               {
+                 return &el->next;
+               }
+               static inline T ***prev_ptr(T *el)
+               {
+                 return &el->prev;
+               }
+             };
+*/
+
+template <typename T, typename B>
+class I_P_List
+{
+  T *first;
+
+  /** Intrusive doubly linked lists are not copyable. */
+  I_P_List(const I_P_List<T,B> &);
+  I_P_List<T,B>& operator=(const I_P_List<T,B> &);
+
+public:
+  I_P_List() : first(NULL) { };
+  inline void empty()      { first= NULL; }
+  inline bool is_empty()   { return (first == NULL); }
+  inline void push_front(T* a)
+  {
+    *B::next_ptr(a)= first;
+    if (first)
+      *B::prev_ptr(first)= B::next_ptr(a);
+    first= a;
+    *B::prev_ptr(a)= &first;
+  }
+  inline void remove(T *a)
+  {
+    T *next= *B::next_ptr(a);
+    if (next)
+      *B::prev_ptr(next)= *B::prev_ptr(a);
+    **B::prev_ptr(a)= next;
+  }
+  inline T* head() { return first; }
+  void swap(I_P_List<T,B> &rhs)
+  {
+    swap_variables(T *, first, rhs.first);
+    if (first)
+      *B::prev_ptr(first)= &first;
+    if (rhs.first)
+      *B::prev_ptr(rhs.first)= &rhs.first;
+  }
+#ifndef _lint
+  friend class I_P_List_iterator<T, B>;
+#endif
+};
+
+
+/**
+   Iterator for I_P_List.
+*/
+
+template <typename T, typename B>
+class I_P_List_iterator
+{
+  I_P_List<T, B> *list;
+  T *current;
+public:
+  I_P_List_iterator(I_P_List<T, B> &a) : list(&a), current(a.first) {}
+  inline T* operator++(int)
+  {
+    T *result= current;
+    if (result)
+      current= *B::next_ptr(current);
+    return result;
+  }
+  inline void rewind()
+  {
+    current= list->first;
+  }
+};
+
+
 /**
   Make a deep copy of each list element.
 
Thread
bk commit into 6.0 tree (dlenev:1.2563) WL#3726dlenev25 Mar