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;
>
>
>
>
>
