rw_lock_x_lock_func_nowait() calls os_thread_get_curr_id() mostly needlessly

Bug #1230220 reported by Laurynas Biveinis
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
MySQL Server
Unknown
Unknown
Percona Server moved to https://jira.percona.com/projects/PS
Fix Released
Wishlist
Laurynas Biveinis
5.1
Won't Fix
Undecided
Unassigned
5.5
Won't Fix
Undecided
Unassigned
5.6
Fix Released
Wishlist
Laurynas Biveinis

Bug Description

[25 Sep 11:04] Laurynas Biveinis

Description:
ibool
rw_lock_x_lock_func_nowait(...)
{
 os_thread_id_t curr_thread = os_thread_get_curr_id();

...attempt to acquire lock by CAS...

 if (success) {
  rw_lock_set_writer_id_and_recursion_flag(lock, TRUE);

 } else if (lock->recursive
     && os_thread_eq(lock->writer_thread, curr_thread)) {
...
 } else {
...
 }
...
}

Thus, curr_thread is only needed if we failed to acquire the lock and it's a recursive lock. os_thread_get_curr_id() is not an inline function, thus it's a call preceding the CAS. And we saw such callstacks in PMP, obviously not in the top positions but still.

How to repeat:
Code analysis, or when you run enough PMP you will see

1 os_thread_get_curr_id(os0thread.cc:98),rw_lock_x_lock_func(sync0rw.cc:1146),pfs_rw_lock_x_lock_func(sync0rw.ic:883),buf_page_init_for_read(sync0rw.ic:883),buf_read_page_low(buf0rea.cc:168),buf_read_page(buf0rea.cc:454),buf_page_get_gen(buf0buf.cc:2698),btr_cur_search_to_nth_level(btr0cur.cc:759),row_ins_clust_index_entry_low(row0ins.cc:2370),row_ins_clust_index_entry(row0ins.cc:2909),row_ins_index_entry(row0ins.cc:2909),row_ins_index_entry_step(row0ins.cc:2909),row_ins(row0ins.cc:2909),row_ins_step(row0ins.cc:2909),row_insert_for_mysql(row0mysql.cc:1315),ha_innobase::write_row(ha_innodb.cc:6922),handler::ha_write_row(handler.cc:7429),ha_partition::write_row(ha_partition.cc:3956),handler::ha_write_row(handler.cc:7429),write_record(sql_insert.cc:1668),mysql_insert(sql_insert.cc:1073),mysql_execute_command(sql_parse.cc:3647),mysql_parse(sql_parse.cc:6477),dispatch_command(sql_parse.cc:1355),do_handle_one_connection(sql_connect.cc:1615),handle_one_connection(sql_connect.cc:1526),start_thread(libpthread.so.0),clone(libc.so.6)

Suggested fix:
After inlining curr_thread variable so that os_thread_get_curr_id() is called in else if only, we never saw such stacktrace again.

Related branches

Revision history for this message
Laurynas Biveinis (laurynas-biveinis) wrote :

Upstream fix in 5.6.16 / 5.7.4.

Revision history for this message
Shahriyar Rzayev (rzayev-sehriyar) wrote :

Percona now uses JIRA for bug reports so this bug report is migrated to: https://jira.percona.com/browse/PS-2429

To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.