List:Commits« Previous MessageNext Message »
From:Christopher Powers Date:July 22 2010 7:33pm
Subject:Re: bzr commit into mysql-trunk-bugfixing branch (marc.alff:3133)
Bug#55462
View as plain text  
Ok to push.

On 7/21/10 7:58 PM, Marc Alff wrote:
> #At file:///home/malff/BZR_TREE/mysql-trunk-bugfixing-55462/ based on
> revid:jonathan.perkin@stripped
>
>   3133 Marc Alff	2010-07-21
>        Bug#55462 Performance schema: reduce the overhead of
> PFS_events_waits::m_wait_class
>
>        This is a performance improvement fix.
>
>        Removed the "volatile" property of PFS_events_waits::m_wait_class.
>        Simplified the code accordingly.
>
>      modified:
>        storage/perfschema/pfs.cc
>        storage/perfschema/pfs_events_waits.cc
>        storage/perfschema/pfs_events_waits.h
>        storage/perfschema/table_events_waits.cc
> === modified file 'storage/perfschema/pfs.cc'
> --- a/storage/perfschema/pfs.cc	2010-07-15 23:44:45 +0000
> +++ b/storage/perfschema/pfs.cc	2010-07-22 00:58:31 +0000
> @@ -1117,7 +1117,6 @@ get_thread_mutex_locker_v1(PSI_mutex_loc
>     }
>     PFS_wait_locker *pfs_locker=&pfs_thread->m_wait_locker_stack
>       [pfs_thread->m_wait_locker_count];
> -  pfs_locker->m_waits_current.m_wait_class= NO_WAIT_CLASS;
>
>     pfs_locker->m_target.m_mutex= pfs_mutex;
>     pfs_locker->m_waits_current.m_thread= pfs_thread;
> @@ -1163,7 +1162,6 @@ get_thread_rwlock_locker_v1(PSI_rwlock_l
>     }
>     PFS_wait_locker *pfs_locker=&pfs_thread->m_wait_locker_stack
>       [pfs_thread->m_wait_locker_count];
> -  pfs_locker->m_waits_current.m_wait_class= NO_WAIT_CLASS;
>
>     pfs_locker->m_target.m_rwlock= pfs_rwlock;
>     pfs_locker->m_waits_current.m_thread= pfs_thread;
> @@ -1222,7 +1220,6 @@ get_thread_cond_locker_v1(PSI_cond_locke
>     }
>     PFS_wait_locker *pfs_locker=&pfs_thread->m_wait_locker_stack
>       [pfs_thread->m_wait_locker_count];
> -  pfs_locker->m_waits_current.m_wait_class= NO_WAIT_CLASS;
>
>     pfs_locker->m_target.m_cond= pfs_cond;
>     pfs_locker->m_waits_current.m_thread= pfs_thread;
> @@ -1267,7 +1264,6 @@ get_thread_table_locker_v1(PSI_table_loc
>     }
>     PFS_wait_locker *pfs_locker=&pfs_thread->m_wait_locker_stack
>       [pfs_thread->m_wait_locker_count];
> -  pfs_locker->m_waits_current.m_wait_class= NO_WAIT_CLASS;
>
>     pfs_locker->m_target.m_table= pfs_table;
>     pfs_locker->m_waits_current.m_thread= pfs_thread;
> @@ -1320,7 +1316,6 @@ get_thread_file_name_locker_v1(PSI_file_
>
>     PFS_wait_locker *pfs_locker=&pfs_thread->m_wait_locker_stack
>       [pfs_thread->m_wait_locker_count];
> -  pfs_locker->m_waits_current.m_wait_class= NO_WAIT_CLASS;
>
>     pfs_locker->m_target.m_file= pfs_file;
>     pfs_locker->m_waits_current.m_thread= pfs_thread;
> @@ -1372,7 +1367,6 @@ get_thread_file_stream_locker_v1(PSI_fil
>     }
>     PFS_wait_locker *pfs_locker=&pfs_thread->m_wait_locker_stack
>       [pfs_thread->m_wait_locker_count];
> -  pfs_locker->m_waits_current.m_wait_class= NO_WAIT_CLASS;
>
>     pfs_locker->m_target.m_file= pfs_file;
>     pfs_locker->m_waits_current.m_thread= pfs_thread;
> @@ -1441,7 +1435,6 @@ get_thread_file_descriptor_locker_v1(PSI
>         }
>         PFS_wait_locker *pfs_locker=&pfs_thread->m_wait_locker_stack
>           [pfs_thread->m_wait_locker_count];
> -      pfs_locker->m_waits_current.m_wait_class= NO_WAIT_CLASS;
>
>         pfs_locker->m_target.m_file= pfs_file;
>         pfs_locker->m_waits_current.m_thread= pfs_thread;
>
> === modified file 'storage/perfschema/pfs_events_waits.cc'
> --- a/storage/perfschema/pfs_events_waits.cc	2010-07-15 23:44:45 +0000
> +++ b/storage/perfschema/pfs_events_waits.cc	2010-07-22 00:58:31 +0000
> @@ -81,26 +81,10 @@ void cleanup_events_waits_history_long(v
>     events_waits_history_long_array= NULL;
>   }
>
> -static void copy_events_waits(PFS_events_waits *dest,
> -                              const PFS_events_waits *source)
> +static inline void copy_events_waits(PFS_events_waits *dest,
> +                                     const PFS_events_waits *source)
>   {
> -  /* m_wait_class must be the first member of PFS_events_waits. */
> -  compile_time_assert(offsetof(PFS_events_waits, m_wait_class) == 0);
> -
> -  char* dest_body= (reinterpret_cast<char*>  (dest)) +
> sizeof(events_waits_class);
> -  const char* source_body= (reinterpret_cast<const char*>  (source))
> -    + sizeof(events_waits_class);
> -
> -  /* See comments in table_events_waits_common::make_row(). */
> -
> -  /* Signal readers they are about to read garbage ... */
> -  dest->m_wait_class= NO_WAIT_CLASS;
> -  /* ... that this can generate. */
> -  memcpy_fixed(dest_body,
> -               source_body,
> -               sizeof(PFS_events_waits) - sizeof(events_waits_class));
> -  /* Signal readers the record is now clean again. */
> -  dest->m_wait_class= source->m_wait_class;
> +  memcpy_fixed(dest, source, sizeof(PFS_events_waits));
>   }
>
>   /**
> @@ -118,9 +102,7 @@ void insert_events_waits_history(PFS_thr
>       causing a potential race condition.
>       We are not testing for this and insert a possibly empty record,
>       to make this thread (the writer) faster.
> -    This is ok, the truncated data will have
> -    wait->m_wait_class == NO_WAIT_CLASS,
> -    which readers of m_waits_history will filter out.
> +    This is ok, the readers of m_waits_history will filter this out.
>     */
>     copy_events_waits(&thread->m_waits_history[index], wait);
>
>
> === modified file 'storage/perfschema/pfs_events_waits.h'
> --- a/storage/perfschema/pfs_events_waits.h	2010-07-15 23:44:45 +0000
> +++ b/storage/perfschema/pfs_events_waits.h	2010-07-22 00:58:31 +0000
> @@ -97,7 +97,7 @@ struct PFS_events_waits
>       - TRUNCATE EVENTS_WAITS_HISTORY
>       - TRUNCATE EVENTS_WAITS_HISTORY_LONG
>     */
> -  volatile events_waits_class m_wait_class;
> +  events_waits_class m_wait_class;
>     /** Executing thread. */
>     PFS_thread *m_thread;
>     /** Instrument metadata. */
>
> === modified file 'storage/perfschema/table_events_waits.cc'
> --- a/storage/perfschema/table_events_waits.cc	2010-07-16 00:06:33 +0000
> +++ b/storage/perfschema/table_events_waits.cc	2010-07-22 00:58:31 +0000
> @@ -217,16 +217,8 @@ void table_events_waits_common::make_row
>       or 8 atomics per recorded event.
>       The problem is that we record a *lot* of events ...
>
> -    Instead, a *dirty* marking is done using m_wait_class.
> -    Using m_wait_class alone does not guarantee anything, it just filters
> -    out most of the bad data.
> -    This is acceptable because this code is garbage-proof,
> -    and won't crash on bad data, only display it,
> -    very rarely (which is accepted).
> -
> -    If a bad record is displayed, it's a very transient failure:
> -    the next select * from EVENTS_WAITS_CURRENT/_HISTORY/_HISTORY_LONG will
> -    show clean data again.
> +    This code is prepared to accept *dirty* records,
> +    and sanitizes all the data before returning a row.
>     */
>
>     m_row.m_thread_internal_id= safe_thread->m_thread_internal_id;
>
>
>
>
>
Thread
bzr commit into mysql-trunk-bugfixing branch (marc.alff:3133) Bug#55462Marc Alff22 Jul
  • Re: bzr commit into mysql-trunk-bugfixing branch (marc.alff:3133)Bug#55462Christopher Powers22 Jul