bzr commit into mysql-5.5-innodb branch (marko.makela:3229) Bug#58226

[marko.makela]

#At file:///home/marko/innobase/dev/mysql2a/5.5-innodb/ based on revid:marko.makela@strippedp7xzdwl8t190

 3229 Marko Mäkelä	2010-11-16
      Bug#58226 Some InnoDB debug checks consume too much CPU time
      
      Do not disable InnoDB inlining when UNIV_DEBUG is defined. The
      inlining is now solely controlled by the preprocessor symbol
      UNIV_MUST_NOT_INLINE and by any compiler options.
      
      mtr_memo_contains(): Add an explicit type conversion from void*, so
      that the function can be compiled by a C++ compiler. Previously, this
      function was never seen by the C++ compiler, because it is only
      present in UNIV_DEBUG builds and InnoDB inlining used to be disabled.
      
      buf_flush_validate_skip(): A wrapper that skips most calls of
      buf_flush_validate_low(). Invoked by debug assertions in
      buf_flush_insert_into_flush_list() and buf_flush_remove().
      
      fil_validate_skip(): A wrapper that skips most calls of
      fil_validate(). Invoked by debug assertions in fil_io() and fil_io_wait().
      
      os_aio_validate_skip(): A wrapper that skips most calls of
      os_aio_validate(). Invoked by debug assertions in
      os_aio_func(), os_aio_windows_handle() and os_aio_simulated_handle.
      
      os_get_os_version(): Only include this function if __WIN__ is defined.
      
      sync_array_deadlock_step(): Slight optimizations. This function is a
      major CPU consumer in UNIV_SYNC_DEBUG builds.

    modified:
      storage/innobase/buf/buf0flu.c
      storage/innobase/fil/fil0fil.c
      storage/innobase/include/mtr0mtr.ic
      storage/innobase/include/os0file.h
      storage/innobase/include/univ.i
      storage/innobase/os/os0file.c
      storage/innobase/sync/sync0arr.c
=== modified file 'storage/innobase/buf/buf0flu.c'
--- a/storage/innobase/buf/buf0flu.c	revid:marko.makela@stripped
+++ b/storage/innobase/buf/buf0flu.c	revid:[e-mail]-20101116123953-62bzmuuymztjajwa
@@ -88,6 +88,34 @@ ibool
 buf_flush_validate_low(
 /*===================*/
 	buf_pool_t*	buf_pool);	/*!< in: Buffer pool instance */
+
+/******************************************************************//**
+Validates the flush list some of the time.
+@return	TRUE if ok or the check was skipped */
+static
+ibool
+buf_flush_validate_skip(
+/*====================*/
+	buf_pool_t*	buf_pool)	/*!< in: Buffer pool instance */
+{
+/** Try buf_flush_validate_low() every this many times */
+# define BUF_FLUSH_VALIDATE_SKIP	23
+
+	/** The buf_flush_validate_low() call skip counter.
+	Use a signed type because of the race condition below. */
+	static int buf_flush_validate_count = BUF_FLUSH_VALIDATE_SKIP;
+
+	/* There is a race condition below, but it does not matter,
+	because this call is only for heuristic purposes. We want to
+	reduce the call frequency of the costly buf_flush_validate_low()
+	check in debug builds. */
+	if (--buf_flush_validate_count > 0) {
+		return(TRUE);
+	}
+
+	buf_flush_validate_count = BUF_FLUSH_VALIDATE_SKIP;
+	return(buf_flush_validate_low(buf_pool));
+}
 #endif /* UNIV_DEBUG || UNIV_BUF_DEBUG */
 
 /******************************************************************//**
@@ -293,7 +321,7 @@ buf_flush_insert_into_flush_list(
 	}
 #endif /* UNIV_DEBUG_VALGRIND */
 #if defined UNIV_DEBUG || defined UNIV_BUF_DEBUG
-	ut_a(buf_flush_validate_low(buf_pool));
+	ut_a(buf_flush_validate_skip(buf_pool));
 #endif /* UNIV_DEBUG || UNIV_BUF_DEBUG */
 
 	buf_flush_list_mutex_exit(buf_pool);
@@ -515,7 +543,7 @@ buf_flush_remove(
 	bpage->oldest_modification = 0;
 
 #if defined UNIV_DEBUG || defined UNIV_BUF_DEBUG
-	ut_a(buf_flush_validate_low(buf_pool));
+	ut_a(buf_flush_validate_skip(buf_pool));
 #endif /* UNIV_DEBUG || UNIV_BUF_DEBUG */
 
 	buf_flush_list_mutex_exit(buf_pool);

=== modified file 'storage/innobase/fil/fil0fil.c'
--- a/storage/innobase/fil/fil0fil.c	revid:marko.makela@strippeddwl8t190
+++ b/storage/innobase/fil/fil0fil.c	revid:marko.makela@stripped
@@ -299,6 +299,34 @@ struct fil_system_struct {
 initialized. */
 static fil_system_t*	fil_system	= NULL;
 
+#ifdef UNIV_DEBUG
+/** Try fil_validate() every this many times */
+# define FIL_VALIDATE_SKIP	17
+
+/******************************************************************//**
+Checks the consistency of the tablespace cache some of the time.
+@return	TRUE if ok or the check was skipped */
+static
+ibool
+fil_validate_skip(void)
+/*===================*/
+{
+	/** The fil_validate() call skip counter. Use a signed type
+	because of the race condition below. */
+	static int fil_validate_count = FIL_VALIDATE_SKIP;
+
+	/* There is a race condition below, but it does not matter,
+	because this call is only for heuristic purposes. We want to
+	reduce the call frequency of the costly fil_validate() check
+	in debug builds. */
+	if (--fil_validate_count > 0) {
+		return(TRUE);
+	}
+
+	fil_validate_count = FIL_VALIDATE_SKIP;
+	return(fil_validate());
+}
+#endif /* UNIV_DEBUG */
 
 /********************************************************************//**
 NOTE: you must call fil_mutex_enter_and_prepare_for_io() first!
@@ -4307,7 +4335,7 @@ fil_io(
 #if (1 << UNIV_PAGE_SIZE_SHIFT) != UNIV_PAGE_SIZE
 # error "(1 << UNIV_PAGE_SIZE_SHIFT) != UNIV_PAGE_SIZE"
 #endif
-	ut_ad(fil_validate());
+	ut_ad(fil_validate_skip());
 #ifndef UNIV_HOTBACKUP
 # ifndef UNIV_LOG_DEBUG
 	/* ibuf bitmap pages must be read in the sync aio mode: */
@@ -4466,7 +4494,7 @@ fil_io(
 
 		mutex_exit(&fil_system->mutex);
 
-		ut_ad(fil_validate());
+		ut_ad(fil_validate_skip());
 	}
 
 	return(DB_SUCCESS);
@@ -4490,7 +4518,7 @@ fil_aio_wait(
 	void*		message;
 	ulint		type;
 
-	ut_ad(fil_validate());
+	ut_ad(fil_validate_skip());
 
 	if (srv_use_native_aio) {
 		srv_set_io_thread_op_info(segment, "native aio handle");
@@ -4521,7 +4549,7 @@ fil_aio_wait(
 
 	mutex_exit(&fil_system->mutex);
 
-	ut_ad(fil_validate());
+	ut_ad(fil_validate_skip());
 
 	/* Do the i/o handling */
 	/* IMPORTANT: since i/o handling for reads will read also the insert

=== modified file 'storage/innobase/include/mtr0mtr.ic'
--- a/storage/innobase/include/mtr0mtr.ic	revid:marko.makela@stripped1116122146-f5ilp7xzdwl8t190
+++ b/storage/innobase/include/mtr0mtr.ic	revid:marko.makela@stripped-62bzmuuymztjajwa
@@ -160,7 +160,7 @@ mtr_memo_contains(
 	while (offset > 0) {
 		offset -= sizeof(mtr_memo_slot_t);
 
-		slot = dyn_array_get_element(memo, offset);
+		slot = (mtr_memo_slot_t*) dyn_array_get_element(memo, offset);
 
 		if ((object == slot->object) && (type == slot->type)) {
 

=== modified file 'storage/innobase/include/os0file.h'
--- a/storage/innobase/include/os0file.h	revid:marko.makela@stripped
+++ b/storage/innobase/include/os0file.h	revid:marko.makela@stripped
@@ -373,6 +373,7 @@ typedef HANDLE	os_file_dir_t;	/*!< direc
 typedef DIR*	os_file_dir_t;	/*!< directory stream */
 #endif
 
+#ifdef __WIN__
 /***********************************************************************//**
 Gets the operating system version. Currently works only on Windows.
 @return	OS_WIN95, OS_WIN31, OS_WINNT, OS_WIN2000, OS_WINXP, OS_WINVISTA,
@@ -381,6 +382,7 @@ UNIV_INTERN
 ulint
 os_get_os_version(void);
 /*===================*/
+#endif /* __WIN__ */
 #ifndef UNIV_HOTBACKUP
 /****************************************************************//**
 Creates the seek mutexes used in positioned reads and writes. */

=== modified file 'storage/innobase/include/univ.i'
--- a/storage/innobase/include/univ.i	revid:marko.makela@strippedom-20101116122146-f5ilp7xzdwl8t190
+++ b/storage/innobase/include/univ.i	revid:marko.makela@stripped953-62bzmuuymztjajwa
@@ -250,7 +250,7 @@ easy way to get it to work. See http://b
 # define UNIV_INTERN
 #endif
 
-#if (!defined(UNIV_DEBUG) && !defined(UNIV_MUST_NOT_INLINE))
+#ifndef UNIV_MUST_NOT_INLINE
 /* Definition for inline version */
 
 #ifdef __WIN__

=== modified file 'storage/innobase/os/os0file.c'
--- a/storage/innobase/os/os0file.c	revid:marko.makela@strippedxzdwl8t190
+++ b/storage/innobase/os/os0file.c	revid:marko.makela@stripped
@@ -302,6 +302,36 @@ UNIV_INTERN ulint	os_n_pending_writes =
 /** Number of pending read operations */
 UNIV_INTERN ulint	os_n_pending_reads = 0;
 
+#ifdef UNIV_DEBUG
+/**********************************************************************//**
+Validates the consistency the aio system some of the time.
+@return	TRUE if ok or the check was skipped */
+UNIV_INTERN
+ibool
+os_aio_validate_skip(void)
+/*======================*/
+{
+/** Try os_aio_validate() every this many times */
+# define OS_AIO_VALIDATE_SKIP	13
+
+	/** The os_aio_validate() call skip counter.
+	Use a signed type because of the race condition below. */
+	static int os_aio_validate_count = OS_AIO_VALIDATE_SKIP;
+
+	/* There is a race condition below, but it does not matter,
+	because this call is only for heuristic purposes. We want to
+	reduce the call frequency of the costly os_aio_validate()
+	check in debug builds. */
+	if (--os_aio_validate_count > 0) {
+		return(TRUE);
+	}
+
+	os_aio_validate_count = OS_AIO_VALIDATE_SKIP;
+	return(os_aio_validate());
+}
+#endif /* UNIV_DEBUG */
+
+#ifdef __WIN__
 /***********************************************************************//**
 Gets the operating system version. Currently works only on Windows.
 @return	OS_WIN95, OS_WIN31, OS_WINNT, OS_WIN2000, OS_WINXP, OS_WINVISTA,
@@ -311,7 +341,6 @@ ulint
 os_get_os_version(void)
 /*===================*/
 {
-#ifdef __WIN__
 	OSVERSIONINFO	  os_info;
 
 	os_info.dwOSVersionInfoSize = sizeof(OSVERSIONINFO);
@@ -340,12 +369,8 @@ os_get_os_version(void)
 		ut_error;
 		return(0);
 	}
-#else
-	ut_error;
-
-	return(0);
-#endif
 }
+#endif /* __WIN__ */
 
 /***********************************************************************//**
 Retrieves the last error number if an error occurs in a file io function.
@@ -4008,7 +4033,7 @@ os_aio_func(
 	ut_ad(n > 0);
 	ut_ad(n % OS_FILE_LOG_BLOCK_SIZE == 0);
 	ut_ad(offset % OS_FILE_LOG_BLOCK_SIZE == 0);
-	ut_ad(os_aio_validate());
+	ut_ad(os_aio_validate_skip());
 #ifdef WIN_ASYNC_IO
 	ut_ad((n & 0xFFFFFFFFUL) == n);
 #endif
@@ -4210,7 +4235,7 @@ os_aio_windows_handle(
 	/* NOTE! We only access constant fields in os_aio_array. Therefore
 	we do not have to acquire the protecting mutex yet */
 
-	ut_ad(os_aio_validate());
+	ut_ad(os_aio_validate_skip());
 	ut_ad(segment < array->n_segments);
 
 	n = array->n_slots / array->n_segments;
@@ -4630,7 +4655,7 @@ restart:
 
 	srv_set_io_thread_op_info(global_segment,
 				  "looking for i/o requests (a)");
-	ut_ad(os_aio_validate());
+	ut_ad(os_aio_validate_skip());
 	ut_ad(segment < array->n_segments);
 
 	n = array->n_slots / array->n_segments;

=== modified file 'storage/innobase/sync/sync0arr.c'
--- a/storage/innobase/sync/sync0arr.c	revid:marko.makela@strippedm-20101116122146-f5ilp7xzdwl8t190
+++ b/storage/innobase/sync/sync0arr.c	revid:marko.makela@stripped953-62bzmuuymztjajwa
@@ -590,9 +590,6 @@ sync_array_deadlock_step(
 	ulint		depth)	/*!< in: recursion depth */
 {
 	sync_cell_t*	new;
-	ibool		ret;
-
-	depth++;
 
 	if (pass != 0) {
 		/* If pass != 0, then we do not know which threads are
@@ -604,7 +601,7 @@ sync_array_deadlock_step(
 
 	new = sync_array_find_thread(arr, thread);
 
-	if (new == start) {
+	if (UNIV_UNLIKELY(new == start)) {
 		/* Stop running of other threads */
 
 		ut_dbg_stop_threads = TRUE;
@@ -616,11 +613,7 @@ sync_array_deadlock_step(
 		return(TRUE);
 
 	} else if (new) {
-		ret = sync_array_detect_deadlock(arr, start, new, depth);
-
-		if (ret) {
-			return(TRUE);
-		}
+		return(sync_array_detect_deadlock(arr, start, new, depth + 1));
 	}
 	return(FALSE);
 }

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