bzr commit into mysql-6.0-falcon-team branch (kevin.lewis:2703) Bug#38568

[Kevin Lewis]

#At file:///C:/Work/bzr/Merge/mysql-6.0-falcon-team/ based on revid:kevin.lewis@stripped

 2703 Kevin Lewis	2009-05-01
      Bug#38568 continued;  This is only a cleanup patch to the previous patch which uses COMPARE_EXCHANGE_POINTER on all calls that update RecordVersion::priorVersion.  Jim Starkey originally intended the priorRecord chain to be accessed in a completely non-blocking manner.  This use of CAS should be unneccessary, in theory.  It is used in order to try and catch any conflicting code path in which two separate threads may want to update a priorVersion pointer simultaneously, or nearly so on separate CPUs.  So these CAS assignments are enclosed in a new macro called  USE_CAS_FOR_PRIOR_VERSION  in this patch.  If there is no perceptible performance cost with this, the CAS code may become permanent and this patch can be undone.
      
      Basically, this patch  allows one to revert to the old direct assignments of priorVersion in order to test the performance.  That is done by commenting out 
      #define USE_CAS_FOR_PRIOR_VERSION

    modified:
      storage/falcon/RecordVersion.cpp
=== modified file 'storage/falcon/RecordVersion.cpp'
--- a/storage/falcon/RecordVersion.cpp	2009-04-16 12:09:15 +0000
+++ b/storage/falcon/RecordVersion.cpp	2009-05-01 18:26:25 +0000
@@ -39,6 +39,19 @@
 static const char THIS_FILE[]=__FILE__;
 #endif
 
+// USE_CAS_FOR_PRIOR_VERSION ma be temporary while we look for  
+// problems... or not if the performance hit is unoticeable.
+// In theory, Record::priorVersion can be updated without CAS 
+// because the code is organized so that only one thread can change 
+// this at a time, even though many threads can be traversing the 
+// prior record chain simultaneously on separate CPUs.  
+// The CycleManagermakes sure that any pointer read from a priorRecord 
+// remains valid while the local copy of that pointer exists.
+// These CAS exchanges protect and identify places where multiple 
+// threads change this pointer at the same time, if possible.
+
+#define USE_CAS_FOR_PRIOR_VERSION
+
 //////////////////////////////////////////////////////////////////////
 // Construction/Destruction
 //////////////////////////////////////////////////////////////////////
@@ -125,8 +138,12 @@ RecordVersion::~RecordVersion()
 {
 	state = recDeleting;
 	Record* prior = priorVersion;
+#ifdef USE_CAS_FOR_PRIOR_VERSION
 	if (!COMPARE_EXCHANGE_POINTER(&priorVersion, prior, NULL))
 		FATAL("~RecordVersion; Unexpected contents in priorVersion\n");
+#else
+	priorVersion = NULL;
+#endif
 
 	// Avoid recursion here. May crash from too many levels
 	// if the same record is updated too often and quickly.
@@ -149,9 +166,13 @@ Record* RecordVersion::releaseNonRecursi
 
 	if (useCount == 1)
 		{
+#ifdef USE_CAS_FOR_PRIOR_VERSION
 		prior = priorVersion;
 		if (!COMPARE_EXCHANGE_POINTER(&priorVersion, prior, NULL))
 			FATAL("RecordVersion::releaseNonRecursive; Unexpected contents in priorVersion\n");
+#else
+		priorVersion = NULL;
+#endif
 		}
 
 	release(REC_HISTORY);
@@ -328,8 +349,13 @@ Record* RecordVersion::clearPriorVersion
 
 	if (prior && prior->useCount == 1)
 		{
+#ifdef USE_CAS_FOR_PRIOR_VERSION
 		if (COMPARE_EXCHANGE_POINTER(&priorVersion, prior, NULL))
 			return prior;
+#else
+		priorVersion = NULL;
+		return prior;
+#endif
 		}
 
 	return NULL;
@@ -340,8 +366,13 @@ void RecordVersion::setPriorVersion(Reco
 	if (newPriorVersion)
 		newPriorVersion->addRef(REC_HISTORY);
 
+#ifdef USE_CAS_FOR_PRIOR_VERSION
 	if (!COMPARE_EXCHANGE_POINTER(&priorVersion, oldPriorVersion, newPriorVersion))
 		FATAL("RecordVersion::setPriorVersion; Unexpected contents in priorVersion\n");
+#else
+	ASSERT(priorVersion == oldPriorVersion);
+	priorVersion = newPriorVersion;
+#endif
 
 	if (oldPriorVersion)
 		oldPriorVersion->release(REC_HISTORY);


Attachment: [text/bzr-bundle] bzr/[e-mail]