From 644aafd5a3d4a9604fea439fd1aae9f0f8fa2806 Mon Sep 17 00:00:00 2001 From: Sunny Bains Date: Tue, 24 May 2016 19:02:55 +0530 Subject: [PATCH] WL#9359 - Code cleanup --- storage/innobase/buf/buf0flu.cc | 5 -- storage/innobase/include/os0thread-create.h | 47 +++---------------- storage/innobase/include/os0thread.h | 6 +++ storage/innobase/include/sync0policy.h | 2 +- storage/innobase/include/sync0rw.ic | 2 +- storage/innobase/include/univ.i | 2 - storage/innobase/lock/lock0lock.cc | 2 +- storage/innobase/lock/lock0wait.cc | 2 +- storage/innobase/log/log0recv.cc | 5 -- storage/innobase/row/row0ftsort.cc | 2 +- storage/innobase/srv/srv0srv.cc | 51 +++++---------------- storage/innobase/srv/srv0start.cc | 10 +--- storage/innobase/sync/sync0arr.cc | 2 +- storage/innobase/sync/sync0debug.cc | 2 +- storage/innobase/sync/sync0rw.cc | 4 +- storage/innobase/ut/ut.cc | 8 ++-- storage/innobase/ut/ut0dbg.cc | 4 +- 17 files changed, 39 insertions(+), 117 deletions(-) diff --git a/storage/innobase/buf/buf0flu.cc b/storage/innobase/buf/buf0flu.cc index fadad1e689a2..c4c6d1801e3b 100644 --- a/storage/innobase/buf/buf0flu.cc +++ b/storage/innobase/buf/buf0flu.cc @@ -3120,11 +3120,6 @@ buf_flush_page_cleaner_coordinator() ulint last_activity = srv_get_activity_count(); ulint last_pages = 0; -#ifdef UNIV_DEBUG_THREAD_CREATION - ib::info() << "page_cleaner thread running, id " - << static_cast(os_thread_get_curr_id()); -#endif /* UNIV_DEBUG_THREAD_CREATION */ - #ifdef UNIV_LINUX /* linux might be able to set different setting for each thread. worth to try to set high priority for page cleaner threads */ diff --git a/storage/innobase/include/os0thread-create.h b/storage/innobase/include/os0thread-create.h index 6e407248d780..416935701af1 100644 --- a/storage/innobase/include/os0thread-create.h +++ b/storage/innobase/include/os0thread-create.h @@ -1,6 +1,6 @@ /***************************************************************************** -Copyright (c) 1995, 2016, Oracle and/or its affiliates. All Rights Reserved. +Copyright (c) 2016, Oracle and/or its affiliates. All Rights Reserved. This program is free software; you can redistribute it and/or modify it under the terms of the GNU General Public License as published by the Free Software @@ -56,8 +56,7 @@ os_thread_close() ib::warn() << "Some (" << os_thread_count.load(std::memory_order_relaxed) - << ") " - << " threads are still active"; + << ") threads are still active"; } } @@ -80,7 +79,7 @@ class Runnable{ @param[in] pfs_key Performance schema key */ explicit Runnable(mysql_pfs_key_t pfs_key) : m_pfs_key(pfs_key) { } #else - Runnable() { } + Runnable(mysql_pfs_key_t) { } #endif /* UNIV_PFS_THREAD */ public: @@ -148,10 +147,9 @@ class Runnable{ #endif /* UNIV_PFS_THREAD */ }; -#ifdef UNIV_PFS_THREAD -/** Creae a detached thread, without args +/** Create a detached thread, without args Note: Captures the local arguments by value [=]. -@para[in] f Callable +@param[in] f Callable @param[in] k PFS thread key */ #define CREATE_THREAD0(f,k) \ do { \ @@ -165,9 +163,9 @@ do { \ t.detach(); \ } while (0); -/** Creae a detached thread. +/** Create a detached thread. Note: Captures the local arguments by value [=]. -@para[in] f Callable +@param[in] f Callable @param[in] k PFS thread key */ #define CREATE_THREAD(f,k,...) \ do { \ @@ -180,37 +178,6 @@ do { \ std::thread t(std::move(task)); \ t.detach(); \ } while (0); -#else /* UNIV_PFS_THREAD */ -/** Creae a detached thread, without args -Note: Captures the local arguments by value [=]. -@para[in] f Callable */ -#define CREATE_THREAD0(f,k) \ -do { \ - std::packaged_task task([=]() \ - { \ - Runnable runnable{}; \ - runnable.run(f); \ - }); \ - /* Note: This throws an exception on failure */ \ - std::thread t(std::move(task)); \ - t.detach(); \ -} while (0); - -/** Creae a detached thread. -Note: Captures the local arguments by value [=]. -@para[in] f Callable */ -#define CREATE_THREAD(f,k,...) \ -do { \ - std::packaged_task task([=]() \ - { \ - Runnable runnable{}; \ - runnable.run(f, __VA_ARGS__); \ - }); \ - /* Note: This throws an exception on failure */ \ - std::thread t(std::move(task)); \ - t.detach(); \ -} while (0); -#endif /* UNIV_PFS_THREAD */ #endif /* !os0thread_create_h */ diff --git a/storage/innobase/include/os0thread.h b/storage/innobase/include/os0thread.h index 960bcb9f6d7b..57796f9ab094 100644 --- a/storage/innobase/include/os0thread.h +++ b/storage/innobase/include/os0thread.h @@ -38,6 +38,12 @@ identifier in Unix is the thread handle itself. os_thread_id_t os_thread_get_curr_id(); +/** Return the thread handle. The purpose of this function is to cast the +native handle to an integer type for consistency +@return the current thread ID cast to an uint64_t */ +#define os_thread_handle() \ + reinterpret_cast(os_thread_get_curr_id()) + /** Compares two thread ids for equality. @param[in] lhs OS thread or thread id @param[in] rhs OS thread or thread id diff --git a/storage/innobase/include/sync0policy.h b/storage/innobase/include/sync0policy.h index ae1f6f18c983..3bd2f785ff0b 100644 --- a/storage/innobase/include/sync0policy.h +++ b/storage/innobase/include/sync0policy.h @@ -105,7 +105,7 @@ class MutexDebug { msg << m_mutex->policy().to_string(); - if (static_cast(m_thread_id) != ULINT_UNDEFINED) { + if (m_thread_id != os_thread_id_t(ULINT_UNDEFINED)) { msg << " addr: " << m_mutex << " acquired: " << locked_from().c_str(); diff --git a/storage/innobase/include/sync0rw.ic b/storage/innobase/include/sync0rw.ic index e176c0acee9a..99702e98ef4c 100644 --- a/storage/innobase/include/sync0rw.ic +++ b/storage/innobase/include/sync0rw.ic @@ -471,7 +471,7 @@ rw_lock_x_lock_func_nowait( /* Relock: this lock_word modification is safe since no other threads can modify (lock, unlock, or reserve) lock_word while there is an exclusive writer and this is the writer thread. */ - if (lock->lock_word == 0 || lock->lock_word == -X_LOCK_HALF_DECR) { + if (lock->lock_word == 0|| lock->lock_word == -X_LOCK_HALF_DECR) { /* There are 1 x-locks */ lock->lock_word -= X_LOCK_DECR; } else if (lock->lock_word <= -X_LOCK_DECR) { diff --git a/storage/innobase/include/univ.i b/storage/innobase/include/univ.i index 8de16fadad6d..2a653fc7d793 100644 --- a/storage/innobase/include/univ.i +++ b/storage/innobase/include/univ.i @@ -563,12 +563,10 @@ functions. */ #ifdef _WIN32 typedef ulint os_thread_ret_t; -# define OS_THREAD_DUMMY_RETURN return(0) # define OS_PATH_SEPARATOR '\\' # define OS_PATH_SEPARATOR_ALT '/' #else typedef void* os_thread_ret_t; -# define OS_THREAD_DUMMY_RETURN return(NULL) # define OS_PATH_SEPARATOR '/' # define OS_PATH_SEPARATOR_ALT '\\' #endif /* _WIN32 */ diff --git a/storage/innobase/lock/lock0lock.cc b/storage/innobase/lock/lock0lock.cc index 79746f8285c3..ceb4bf12df71 100644 --- a/storage/innobase/lock/lock0lock.cc +++ b/storage/innobase/lock/lock0lock.cc @@ -426,7 +426,7 @@ lock_sys_create( ulint lock_sys_sz; lock_sys_sz = sizeof(*lock_sys) - + srv_max_n_threads * sizeof(srv_slot_t); + + srv_max_n_threads * sizeof(srv_slot_t); lock_sys = static_cast(ut_zalloc_nokey(lock_sys_sz)); diff --git a/storage/innobase/lock/lock0wait.cc b/storage/innobase/lock/lock0wait.cc index fe6f23769b6e..40b8166bbcd5 100644 --- a/storage/innobase/lock/lock0wait.cc +++ b/storage/innobase/lock/lock0wait.cc @@ -167,7 +167,7 @@ lock_wait_table_reserve_slot( } } - ib::error() << "There appear to be " << srv_max_n_threads<< " user" + ib::error() << "There appear to be " << srv_max_n_threads << " user" " threads currently waiting inside InnoDB, which is the upper" " limit. Cannot continue operation. Before aborting, we print" " a list of waiting threads."; diff --git a/storage/innobase/log/log0recv.cc b/storage/innobase/log/log0recv.cc index f2dce778663a..f6e0ae00f7e7 100644 --- a/storage/innobase/log/log0recv.cc +++ b/storage/innobase/log/log0recv.cc @@ -836,11 +836,6 @@ recv_writer_thread() { ut_ad(!srv_read_only_mode); -#ifdef UNIV_DEBUG_THREAD_CREATION - ib::info() << "recv_writer thread running, id " - << std::this_thread::native_handle; -#endif /* UNIV_DEBUG_THREAD_CREATION */ - /* The code flow is as follows: Step 1: In recv_recovery_from_checkpoint_start(). Step 2: This recv_writer thread is started. diff --git a/storage/innobase/row/row0ftsort.cc b/storage/innobase/row/row0ftsort.cc index d082e86d4d1e..e37b0289b645 100644 --- a/storage/innobase/row/row0ftsort.cc +++ b/storage/innobase/row/row0ftsort.cc @@ -44,7 +44,7 @@ Created 10/13/2010 Jimmy Yang b[N] = row_merge_read_rec( \ block[N], buf[N], b[N], index, \ fd[N], &foffs[N], &mrec[N], offsets[N]); \ - if (!b[N]) { \ + if (UNIV_UNLIKELY(!b[N])) { \ if (mrec[N]) { \ goto exit; \ } \ diff --git a/storage/innobase/srv/srv0srv.cc b/storage/innobase/srv/srv0srv.cc index 18eae7eb6747..16b42a4457be 100644 --- a/storage/innobase/srv/srv0srv.cc +++ b/storage/innobase/srv/srv0srv.cc @@ -412,8 +412,8 @@ ib_mutex_t srv_misc_tmpfile_mutex; /** Temporary file for miscellanous diagnostic output */ FILE* srv_misc_tmpfile; -static ulint srv_main_thread_process_no = 0; -static ulint srv_main_thread_id = 0; +static ulint srv_main_thread_process_no = 0; +static os_thread_id_t srv_main_thread_id = 0; /* The following counts are used by the srv_master_thread. */ @@ -1265,13 +1265,14 @@ srv_printf_innodb_monitor( n_reserved); } - fprintf(file, - "Process ID=" ULINTPF - ", Main thread ID=" ULINTPF - ", state: %s\n", - srv_main_thread_process_no, - srv_main_thread_id, - srv_main_thread_op_info); + std::ostringstream msg; + + msg << "Process ID=" << srv_main_thread_process_no + << ", Main thread ID=" << srv_main_thread_id + << " , state" << srv_main_thread_op_info; + + fprintf(file, "%s\n", msg.str().c_str()); + fprintf(file, "Number of rows inserted " ULINTPF ", updated " ULINTPF @@ -1596,11 +1597,6 @@ srv_error_monitor_thread() old_lsn = srv_start_lsn; -#ifdef UNIV_DEBUG_THREAD_CREATION - ib::info() << "Error monitor thread starts, id " - << static_cast(os_thread_get_curr_id()); -#endif /* UNIV_DEBUG_THREAD_CREATION */ - srv_error_monitor_active = TRUE; loop: @@ -2236,13 +2232,8 @@ srv_master_thread() ut_ad(!srv_read_only_mode); -#ifdef UNIV_DEBUG_THREAD_CREATION - ib::info() << "Master thread starts, id " - << static_cast(os_thread_get_curr_id()); -#endif /* UNIV_DEBUG_THREAD_CREATION */ - srv_main_thread_process_no = os_proc_get_number(); - srv_main_thread_id = static_cast(os_thread_get_curr_id()); + srv_main_thread_id = os_thread_get_curr_id(); slot = srv_reserve_slot(SRV_MASTER); ut_a(slot == srv_sys->sys_threads); @@ -2366,11 +2357,6 @@ srv_worker_thread() THD* thd = create_thd(false, true, true, srv_worker_thread_key); -#ifdef UNIV_DEBUG_THREAD_CREATION - ib::info() << "Worker thread starting, id " - << static_cast(os_thread_get_curr_id()); -#endif /* UNIV_DEBUG_THREAD_CREATION */ - slot = srv_reserve_slot(SRV_WORKER); ut_a(srv_n_purge_threads > 1); @@ -2412,11 +2398,6 @@ srv_worker_thread() rw_lock_x_unlock(&purge_sys->latch); -#ifdef UNIV_DEBUG_THREAD_CREATION - ib::info() << "Purge worker thread exiting, id " - << static_cast(os_thread_get_curr_id()); -#endif /* UNIV_DEBUG_THREAD_CREATION */ - destroy_thd(thd); } @@ -2631,11 +2612,6 @@ srv_purge_coordinator_thread() rw_lock_x_unlock(&purge_sys->latch); -#ifdef UNIV_DEBUG_THREAD_CREATION - ib::info() << "Purge coordinator thread created, id " - << static_cast(os_thread_get_curr_id()); -#endif /* UNIV_DEBUG_THREAD_CREATION */ - slot = srv_reserve_slot(SRV_PURGE); ulint rseg_history_len = trx_sys->rseg_history_len; @@ -2715,11 +2691,6 @@ srv_purge_coordinator_thread() rw_lock_x_unlock(&purge_sys->latch); -#ifdef UNIV_DEBUG_THREAD_CREATION - ib::info() << "Purge coordinator exiting, id " - << static_cast(os_thread_get_curr_id()); -#endif /* UNIV_DEBUG_THREAD_CREATION */ - /* Ensure that all the worker threads quit. */ if (srv_n_purge_threads > 1) { srv_release_threads(SRV_WORKER, srv_n_purge_threads - 1); diff --git a/storage/innobase/srv/srv0start.cc b/storage/innobase/srv/srv0start.cc index 3825424070a7..3f311d67e561 100644 --- a/storage/innobase/srv/srv0start.cc +++ b/storage/innobase/srv/srv0start.cc @@ -268,14 +268,10 @@ srv_file_check_mode( #ifndef UNIV_HOTBACKUP /** I/o-handler thread function. @param[in] segment The AIO segment the thread will work on */ +static void io_handler_thread(ulint segment) { -#ifdef UNIV_DEBUG_THREAD_CREATION - ib::info() << "Io handler thread " << segment << " starts, id " - << os_thread_pf(os_thread_get_curr_id()); -#endif /* UNIV_DEBUG_THREAD_CREATION */ - while (srv_shutdown_state != SRV_SHUTDOWN_EXIT_THREADS || buf_page_cleaner_is_active || !os_aio_all_slots_free()) { @@ -1560,15 +1556,12 @@ srv_start( /* Create i/o-handler threads: */ -#ifdef UNIV_PFS_THREAD /* For read only mode, we don't need ibuf and log I/O thread. Please see innobase_start_or_create_for_mysql() */ ulint start = (srv_read_only_mode) ? 0 : 2; -#endif /* UNIV_PFS_THREAD */ for (ulint t = 0; t < srv_n_file_io_threads; ++t) { -#ifdef UNIV_PFS_THREAD if (t < start) { if (t == 0) { CREATE_THREAD( @@ -1598,7 +1591,6 @@ srv_start( io_handler_thread, io_handler_thread_key, t); } -#endif /* UNIV_PFS_THREAD */ } /* Even in read-only mode there could be flush job generated by diff --git a/storage/innobase/sync/sync0arr.cc b/storage/innobase/sync/sync0arr.cc index 9dd7a96f1033..097588f7f929 100644 --- a/storage/innobase/sync/sync0arr.cc +++ b/storage/innobase/sync/sync0arr.cc @@ -573,7 +573,7 @@ sync_array_cell_print( fprintf(file, "a writer (thread id %lu) has" " reserved it in mode %s", - (ulong) static_cast(rwlock->writer_thread), + reinterpret_cast(rwlock->writer_thread), writer == RW_LOCK_X ? " exclusive\n" : writer == RW_LOCK_SX ? " SX\n" : " wait exclusive\n"); diff --git a/storage/innobase/sync/sync0debug.cc b/storage/innobase/sync/sync0debug.cc index 510eb27d8401..ca9b4b76c044 100644 --- a/storage/innobase/sync/sync0debug.cc +++ b/storage/innobase/sync/sync0debug.cc @@ -584,7 +584,7 @@ LatchDebug::crash( get_level_name(latched->m_level); ib::error() - << "Thread " << static_cast(os_thread_get_curr_id()) + << "Thread " << os_thread_get_curr_id() << " already owns a latch " << sync_latch_get_name(latch->m_id) << " at level" << " " << latched->m_level << " (" << latch_level_name diff --git a/storage/innobase/sync/sync0rw.cc b/storage/innobase/sync/sync0rw.cc index 6191182fc29f..a9e2e38d98e0 100644 --- a/storage/innobase/sync/sync0rw.cc +++ b/storage/innobase/sync/sync0rw.cc @@ -1231,7 +1231,7 @@ rw_lock_debug_print( ulint rwt = info->lock_type; fprintf(f, "Locked: thread %lu file %s line %lu ", - static_cast(static_cast(info->thread_id)), + static_cast(info->thread_id), sync_basename(info->file_name), static_cast(info->line)); @@ -1300,7 +1300,7 @@ rw_lock_t::to_string() const std::ostringstream msg; msg << "RW-LATCH: " - << "thread id " << static_cast(os_thread_get_curr_id()) + << "thread id " << os_thread_get_curr_id() << " addr: " << this << " Locked from: " << locked_from().c_str(); diff --git a/storage/innobase/ut/ut.cc b/storage/innobase/ut/ut.cc index de5d322b418c..f3f7ddd76411 100644 --- a/storage/innobase/ut/ut.cc +++ b/storage/innobase/ut/ut.cc @@ -119,9 +119,7 @@ ut_print_timestamp( /*===============*/ FILE* file) /*!< in: file where to print */ { - ulonglong thread_id; - - thread_id = static_cast(os_thread_get_curr_id()); + auto thread_id = os_thread_handle(); #ifdef _WIN32 SYSTEMTIME cal_tm; @@ -135,7 +133,7 @@ ut_print_timestamp( (int) cal_tm.wHour, (int) cal_tm.wMinute, (int) cal_tm.wSecond, - thread_id); + (ulonglong) thread_id); #else struct tm* cal_tm_ptr; time_t tm; @@ -151,7 +149,7 @@ ut_print_timestamp( cal_tm_ptr->tm_hour, cal_tm_ptr->tm_min, cal_tm_ptr->tm_sec, - thread_id); + (ulonglong) thread_id); #endif /* _WIN32 */ } diff --git a/storage/innobase/ut/ut0dbg.cc b/storage/innobase/ut/ut0dbg.cc index 8d3ccf3f3446..3d0b505469b7 100644 --- a/storage/innobase/ut/ut0dbg.cc +++ b/storage/innobase/ut/ut0dbg.cc @@ -38,11 +38,11 @@ ut_dbg_assertion_failed( ulint line) /*!< in: line number of the assertion */ { sql_print_error("InnoDB: Assertion failure: %s:" ULINTPF "%s%s\n" - "InnoDB: thread " ULINTPF, + "InnoDB: thread " UINT64PF, innobase_basename(file), line, expr != nullptr ? ":" : "", expr != nullptr ? expr : "", - static_cast(os_thread_get_curr_id())); + os_thread_handle()); fputs("InnoDB: We intentionally generate a memory trap.\n" "InnoDB: Submit a detailed bug report"