Merge lp:~percona-dev/percona-server/atomic-fixes-55 into lp:percona-server/5.5

Proposed by Laurynas Biveinis
Status: Superseded
Proposed branch: lp:~percona-dev/percona-server/atomic-fixes-55
Merge into: lp:percona-server/5.5
Prerequisite: lp:~percona-dev/percona-server/release-5.5.12-20.3
Diff against target: 166 lines (+34/-91)
1 file modified
response_time_distribution.patch (+34/-91)
To merge this branch: bzr merge lp:~percona-dev/percona-server/atomic-fixes-55
Reviewer Review Type Date Requested Status
Stewart Smith (community) Needs Fixing
Valentine Gostev qa Pending
Review via email: mp+63955@code.launchpad.net

This proposal has been superseded by a proposal from 2011-06-21.

Description of the change

Fix LP bug 737947: unportable atomic constructs break build on various platforms.

The fix is to remove query_response_time.cc-specific atomic code, and use MySQL primitives in my_atomic.h. This way the portability is not limited in any way by the patch as well as minimizing future maintenance burden.

Testing results:
x86_64 Linux: OK,
FreeBSD 8.2 i386: OK,
Solaris 11 x86_64: build fails due to unrelated error, the Solaris atomics are correctly detected and compiled..

Please ignore the prerequisite branch, I will merge before it is merged, I'm leaving it in to show only the relevant diffs.

To post a comment you must log in.
Revision history for this message
Laurynas Biveinis (laurynas-biveinis) wrote :

Please ignore the apparent conflict in series.

Revision history for this message
Stewart Smith (stewart) wrote :

Please remerge with trunk, we have 5.5.13 in there now.

review: Needs Fixing

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'response_time_distribution.patch'
--- response_time_distribution.patch 2011-06-16 01:19:11 +0000
+++ response_time_distribution.patch 2011-06-21 02:38:08 +0000
@@ -178,12 +178,8 @@
178diff -ruN /dev/null b/sql/query_response_time.cc178diff -ruN /dev/null b/sql/query_response_time.cc
179--- /dev/null 1970-01-01 00:00:00.000000000 +0000179--- /dev/null 1970-01-01 00:00:00.000000000 +0000
180+++ b/sql/query_response_time.cc 2011-04-10 00:27:11.000000000 +0400180+++ b/sql/query_response_time.cc 2011-04-10 00:27:11.000000000 +0400
181@@ -0,0 +1,367 @@181@@ -0,0 +1,310 @@
182+#include "mysql_version.h"182+#include "mysql_version.h"
183+#ifdef __FreeBSD__
184+#include <sys/types.h>
185+#include <machine/atomic.h>
186+#endif // __FreeBSD__
187+#include "my_global.h"183+#include "my_global.h"
188+#ifdef HAVE_RESPONSE_TIME_DISTRIBUTION184+#ifdef HAVE_RESPONSE_TIME_DISTRIBUTION
189+#include "mysql_com.h"185+#include "mysql_com.h"
@@ -337,98 +333,39 @@
337+ }333+ }
338+ buffer[result_length]= 0;334+ buffer[result_length]= 0;
339+}335+}
340+#ifdef __x86_64__
341+typedef uint64 TimeCounter;
342+void add_time_atomic(TimeCounter* counter, uint64 time)
343+{
344+ __sync_fetch_and_add(counter,time);
345+}
346+#endif // __x86_64__
347+#ifdef __i386__
348+inline uint32 get_high(uint64 value)
349+{
350+ return ((value >> 32) << 32);
351+}
352+inline uint32 get_low(uint64 value)
353+{
354+ return ((value << 32) >> 32);
355+}
356+#ifdef __FreeBSD__
357+inline bool compare_and_swap(volatile uint32 *target, uint32 old, uint32 new_value)
358+{
359+ return atomic_cmpset_32(target,old,new_value);
360+}
361+#else // __FreeBSD__
362+inline bool compare_and_swap(volatile uint32* target, uint32 old, uint32 new_value)
363+{
364+ return __sync_bool_compare_and_swap(target,old,new_value);
365+}
366+#endif // __FreeBSD__
367+class TimeCounter
368+{
369+public:
370+ TimeCounter& operator=(uint64 time)
371+ {
372+ this->m_high= get_high(time);
373+ this->m_low= get_low(time);
374+ return *this;
375+ }
376+ operator uint64() const
377+ {
378+ return ((static_cast<uint64>(m_high) << 32) + static_cast<uint64>(m_low));
379+ }
380+ void add(uint64 time)
381+ {
382+ uint32 time_high = get_high(time);
383+ uint32 time_low = get_low(time);
384+ uint64 time_low64= time_low;
385+ while(true)
386+ {
387+ uint32 old_low= this->m_low;
388+ uint64 old_low64= old_low;
389+
390+ uint64 new_low64= old_low64 + time_low64;
391+ uint32 new_low= (get_low(new_low64));
392+ bool add_high= (get_high(new_low64) != 0);
393+
394+ if(!compare_and_swap(&m_low,old_low,new_low))
395+ {
396+ continue;
397+ }
398+ if(add_high)
399+ {
400+ ++time_high;
401+ }
402+ if(time_high > 0)
403+ {
404+ __sync_fetch_and_add(&m_high,time_high);
405+ }
406+ break;
407+ }
408+ }
409+private:
410+ uint32 m_low;
411+ uint32 m_high;
412+};
413+void add_time_atomic(TimeCounter* counter, uint64 time)
414+{
415+ counter->add(time);
416+}
417+#endif // __i386__
418+336+
419+class time_collector337+class time_collector
420+{338+{
421+public:339+public:
422+ time_collector(utility& u) : m_utility(&u)340+ time_collector(utility& u) : m_utility(&u)
423+ {341+ {
424+ }342+ my_atomic_rwlock_init(&time_collector_lock);
425+ uint32 count(uint index) const { return m_count[index]; }343+ }
426+ uint64 total(uint index) const { return m_total[index]; }344+ ~time_collector()
345+ {
346+ my_atomic_rwlock_destroy(&time_collector_lock);
347+ }
348+ uint32 count(uint index) const
349+ {
350+ my_atomic_rwlock_rdlock(&time_collector_lock);
351+ uint32 result= my_atomic_load32((volatile int32*)&m_count[index]);
352+ my_atomic_rwlock_rdunlock(&time_collector_lock);
353+ return result;
354+ }
355+ uint64 total(uint index) const
356+ {
357+ my_atomic_rwlock_rdlock(&time_collector_lock);
358+ uint64 result= my_atomic_load64((volatile int64*)&m_total[index]);
359+ my_atomic_rwlock_rdunlock(&time_collector_lock);
360+ return result;
361+ }
427+public:362+public:
428+ void flush()363+ void flush()
429+ {364+ {
430+ memset(&m_count,0,sizeof(m_count));365+ my_atomic_rwlock_wrlock(&time_collector_lock);
366+ memset((void*)&m_count,0,sizeof(m_count));
431+ memset((void*)&m_total,0,sizeof(m_total));367+ memset((void*)&m_total,0,sizeof(m_total));
368+ my_atomic_rwlock_wrunlock(&time_collector_lock);
432+ }369+ }
433+ void collect(uint64 time)370+ void collect(uint64 time)
434+ {371+ {
@@ -437,16 +374,22 @@
437+ {374+ {
438+ if(m_utility->bound(i) > time)375+ if(m_utility->bound(i) > time)
439+ {376+ {
440+ __sync_fetch_and_add(&(m_count[i]),(uint32)1);377+ my_atomic_rwlock_wrlock(&time_collector_lock);
441+ add_time_atomic(&(m_total[i]),time);378+ my_atomic_add32((volatile int32*)(&m_count[i]), 1);
379+ my_atomic_add64((volatile int64*)(&m_total[i]), time);
380+ my_atomic_rwlock_wrunlock(&time_collector_lock);
442+ break;381+ break;
443+ }382+ }
444+ }383+ }
445+ }384+ }
446+private:385+private:
447+ utility* m_utility;386+ utility* m_utility;
448+ uint32 m_count[OVERALL_POWER_COUNT + 1];387+ /* The lock for atomic operations on m_count and m_total. Only actually
449+ TimeCounter m_total[OVERALL_POWER_COUNT + 1];388+ used on architectures that do not have atomic implementation of atomic
389+ operations. */
390+ my_atomic_rwlock_t time_collector_lock;
391+ volatile uint32 m_count[OVERALL_POWER_COUNT + 1];
392+ volatile uint64 m_total[OVERALL_POWER_COUNT + 1];
450+};393+};
451+394+
452+class collector395+class collector

Subscribers

People subscribed via source and target branches