Skip to content

Commit

Permalink
MDEV-22930 Unnecessary contention on rw_lock_list_mutex in ibuf_dummy…
Browse files Browse the repository at this point in the history 
…_index_create()

1. Do not initialize dict_table_t::stats_latch in ibuf
2. Remove overengineering in GenericPolicy to speed up things

dict_mem_table_create(): add new argument init_stats_latch

ibuf_dummy_index_create(): do not initialize dict_table_t::stats_latch

GenericPolicy: add new members m_filename and m_line

sync_file_create_register()
sync_file_created_deregister()
sync_file_created_get()
CreateTracker: remove

rw_lock_t::created: a new debug member
  • Loading branch information
@kevgs
kevgs committed Jul 8, 2020
1 parent 1ae008d commit 0e86254
Show file tree
Hide file tree
Showing 8 changed files with 61 additions and 222 deletions.
37 changes: 24 additions & 13 deletions storage/innobase/dict/dict0mem.cc
View file
Expand Up @@ -124,19 +124,25 @@ bool dict_col_t::same_encoding(uint16_t a, uint16_t b)
return false;
}

/**********************************************************************//**
Creates a table memory object.
/** Creates a table memory object.
@param[in] name table name
@param[in] space tablespace
@param[in] n_cols total number of columns including virtual and
non-virtual columns
@param[in] n_v_cols number of virtual columns
@param[in] flags table flags
@param[in] flags2 table flags2
@param[in] init_stats_latch whether to init the stats latch
@return own: table object */
dict_table_t*
dict_mem_table_create(
/*==================*/
const char* name, /*!< in: table name */
fil_space_t* space, /*!< in: tablespace */
ulint n_cols, /*!< in: total number of columns including
virtual and non-virtual columns */
ulint n_v_cols,/*!< in: number of virtual columns */
ulint flags, /*!< in: table flags */
ulint flags2) /*!< in: table flags2 */
const char* name,
fil_space_t* space,
ulint n_cols,
ulint n_v_cols,
ulint flags,
ulint flags2,
bool init_stats_latch)
{
dict_table_t* table;
mem_heap_t* heap;
Expand Down Expand Up @@ -204,8 +210,11 @@ dict_mem_table_create(
new(&table->foreign_set) dict_foreign_set();
new(&table->referenced_set) dict_foreign_set();

rw_lock_create(dict_table_stats_key, &table->stats_latch,
SYNC_INDEX_TREE);
if (init_stats_latch) {
rw_lock_create(dict_table_stats_key, &table->stats_latch,
SYNC_INDEX_TREE);
table->stats_latch_inited = true;
}

return(table);
}
Expand Down Expand Up @@ -249,7 +258,9 @@ dict_mem_table_free(

UT_DELETE(table->s_cols);

rw_lock_free(&table->stats_latch);
if (table->stats_latch_inited) {
rw_lock_free(&table->stats_latch);
}

mem_heap_free(table->heap);
}
Expand Down
2 changes: 1 addition & 1 deletion storage/innobase/ibuf/ibuf0ibuf.cc
View file
Expand Up @@ -1309,7 +1309,7 @@ ibuf_dummy_index_create(
dict_index_t* index;

table = dict_mem_table_create("IBUF_DUMMY", NULL, n, 0,
comp ? DICT_TF_COMPACT : 0, 0);
comp ? DICT_TF_COMPACT : 0, 0, false);

index = dict_mem_index_create(table, "IBUF_DUMMY", 0, n);

Expand Down
31 changes: 16 additions & 15 deletions storage/innobase/include/dict0mem.h
View file
Expand Up @@ -297,22 +297,21 @@ parent table will fail, and user has to drop excessive foreign constraint
before proceeds. */
#define FK_MAX_CASCADE_DEL 15

/**********************************************************************//**
Creates a table memory object.
/** Creates a table memory object.
@param[in] name table name
@param[in] space tablespace
@param[in] n_cols total number of columns including virtual and
non-virtual columns
@param[in] n_v_cols number of virtual columns
@param[in] flags table flags
@param[in] flags2 table flags2
@param[in] init_stats_latch whether to init the stats latch
@return own: table object */
dict_table_t*
dict_mem_table_create(
/*==================*/
const char* name, /*!< in: table name */
fil_space_t* space, /*!< in: tablespace */
ulint n_cols, /*!< in: total number of columns
including virtual and non-virtual
columns */
ulint n_v_cols, /*!< in: number of virtual columns */
ulint flags, /*!< in: table flags */
ulint flags2); /*!< in: table flags2 */
/****************************************************************//**
Free a table memory object. */
dict_table_t *dict_mem_table_create(const char *name, fil_space_t *space,
ulint n_cols, ulint n_v_cols, ulint flags,
ulint flags2, bool init_stats_latch= true);
/****************************************************************/ /**
Free a table memory object. */
void
dict_mem_table_free(
/*================*/
Expand Down Expand Up @@ -2146,6 +2145,8 @@ struct dict_table_t {
performance reasons. */
rw_lock_t stats_latch;

bool stats_latch_inited= false;

/** TRUE if statistics have been calculated the first time after
database startup or table creation. */
unsigned stat_initialized:1;
Expand Down
15 changes: 11 additions & 4 deletions storage/innobase/include/sync0policy.h
View file
Expand Up @@ -220,7 +220,8 @@ struct GenericPolicy

meta.get_counter()->single_register(&m_count);

sync_file_created_register(this, filename, uint16_t(line));
m_filename = filename;
m_line = line;
}

/** Called when the mutex is destroyed. */
Expand All @@ -230,8 +231,6 @@ struct GenericPolicy
latch_meta_t& meta = sync_latch_get_meta(m_id);

meta.get_counter()->single_deregister(&m_count);

sync_file_created_deregister(this);
}

/** Called after a successful mutex acquire.
Expand Down Expand Up @@ -272,13 +271,21 @@ struct GenericPolicy

/** @return the string representation */
std::string to_string() const
{ return sync_mutex_to_string(get_id(), sync_file_created_get(this)); }
{
return sync_mutex_to_string(get_id(),
std::string(m_filename)
.append(":")
.append(std::to_string(m_line)));
}

#ifdef UNIV_DEBUG
MutexDebug<Mutex> context;
#endif

private:
const char *m_filename;
uint32_t m_line;

/** The user visible counters, registered with the meta-data. */
latch_meta_t::CounterType::Count m_count;

Expand Down
2 changes: 2 additions & 0 deletions storage/innobase/include/sync0rw.h
View file
Expand Up @@ -567,6 +567,8 @@ struct rw_lock_t :
#endif /* UNIV_DEBUG */
public ilist_node<>
{
ut_d(bool created= false;)

/** Holds the state of the lock. */
Atomic_relaxed<int32_t> lock_word;

Expand Down
21 changes: 0 additions & 21 deletions storage/innobase/include/sync0types.h
View file
Expand Up @@ -941,27 +941,6 @@ sync_latch_get_name(latch_level_t level);
const char*
sync_basename(const char* filename);

/** Register a latch, called when it is created
@param[in] ptr Latch instance that was created
@param[in] filename Filename where it was created
@param[in] line Line number in filename */
void
sync_file_created_register(
const void* ptr,
const char* filename,
uint16_t line);

/** Deregister a latch, called when it is destroyed
@param[in] ptr Latch to be destroyed */
void
sync_file_created_deregister(const void* ptr);

/** Get the string where the file was created. Its format is "name:line"
@param[in] ptr Latch instance
@return created information or "" if can't be found */
std::string
sync_file_created_get(const void* ptr);

#ifdef UNIV_DEBUG

/** All (ordered) latches, used in debugging, must derive from this class. */
Expand Down
169 changes: 1 addition & 168 deletions storage/innobase/sync/sync0debug.cc
View file
Expand Up @@ -38,7 +38,7 @@ Created 2012-08-21 Sunny Bains
#include <vector>
#include <string>
#include <algorithm>
#include <iostream>
#include <map>

#ifdef UNIV_DEBUG

Expand Down Expand Up @@ -1506,173 +1506,6 @@ sync_latch_meta_destroy()
latch_meta.clear();
}

/** Track mutex file creation name and line number. This is to avoid storing
{ const char* name; uint16_t line; } in every instance. This results in the
sizeof(Mutex) > 64. We use a lookup table to store it separately. Fetching
the values is very rare, only required for diagnostic purposes. And, we
don't create/destroy mutexes that frequently. */
struct CreateTracker {

/** Constructor */
CreateTracker()
UNIV_NOTHROW
{
m_mutex.init();
}

/** Destructor */
~CreateTracker()
UNIV_NOTHROW
{
ut_ad(m_files.empty());

m_mutex.destroy();
}

/** Register where the latch was created
@param[in] ptr Latch instance
@param[in] filename Where created
@param[in] line Line number in filename */
void register_latch(
const void* ptr,
const char* filename,
uint16_t line)
UNIV_NOTHROW
{
m_mutex.enter();

Files::iterator lb = m_files.lower_bound(ptr);

ut_ad(lb == m_files.end()
|| m_files.key_comp()(ptr, lb->first));

typedef Files::value_type value_type;

m_files.insert(lb, value_type(ptr, File(filename, line)));

m_mutex.exit();
}

/** Deregister a latch - when it is destroyed
@param[in] ptr Latch instance being destroyed */
void deregister_latch(const void* ptr)
UNIV_NOTHROW
{
m_mutex.enter();

Files::iterator lb = m_files.lower_bound(ptr);

ut_ad(lb != m_files.end()
&& !(m_files.key_comp()(ptr, lb->first)));

m_files.erase(lb);

m_mutex.exit();
}

/** Get the create string, format is "name:line"
@param[in] ptr Latch instance
@return the create string or "" if not found */
std::string get(const void* ptr)
UNIV_NOTHROW
{
m_mutex.enter();

std::string created;

Files::iterator lb = m_files.lower_bound(ptr);

if (lb != m_files.end()
&& !(m_files.key_comp()(ptr, lb->first))) {

std::ostringstream msg;

msg << lb->second.m_name << ":" << lb->second.m_line;

created = msg.str();
}

m_mutex.exit();

return(created);
}

private:
/** For tracking the filename and line number */
struct File {

/** Constructor */
File() UNIV_NOTHROW : m_name(), m_line() { }

/** Constructor
@param[in] name Filename where created
@param[in] line Line number where created */
File(const char* name, uint16_t line)
UNIV_NOTHROW
:
m_name(sync_basename(name)),
m_line(line)
{
/* No op */
}

/** Filename where created */
std::string m_name;

/** Line number where created */
uint16_t m_line;
};

/** Map the mutex instance to where it was created */
typedef std::map<
const void*,
File,
std::less<const void*>,
ut_allocator<std::pair<const void* const, File> > >
Files;

typedef OSMutex Mutex;

/** Mutex protecting m_files */
Mutex m_mutex;

/** Track the latch creation */
Files m_files;
};

/** Track latch creation location. For reducing the size of the latches */
static CreateTracker create_tracker;

/** Register a latch, called when it is created
@param[in] ptr Latch instance that was created
@param[in] filename Filename where it was created
@param[in] line Line number in filename */
void
sync_file_created_register(
const void* ptr,
const char* filename,
uint16_t line)
{
create_tracker.register_latch(ptr, filename, line);
}

/** Deregister a latch, called when it is destroyed
@param[in] ptr Latch to be destroyed */
void
sync_file_created_deregister(const void* ptr)
{
create_tracker.deregister_latch(ptr);
}

/** Get the string where the file was created. Its format is "name:line"
@param[in] ptr Latch instance
@return created information or "" if can't be found */
std::string
sync_file_created_get(const void* ptr)
{
return(create_tracker.get(ptr));
}

/** Initializes the synchronization data structures. */
void
sync_check_init()
Expand Down

0 comments on commit 0e86254

Please sign in to comment.