| Bug #70417 | rw_lock_x_lock_func_nowait() calls os_thread_get_curr_id() mostly needlessly | ||
|---|---|---|---|
| Submitted: | 25 Sep 2013 11:04 | Modified: | 3 Feb 2014 10:48 |
| Reporter: | Laurynas Biveinis (OCA) | Email Updates: | |
| Status: | Closed | Impact on me: | |
| Category: | MySQL Server: InnoDB storage engine | Severity: | S5 (Performance) |
| Version: | 5.6 | OS: | Any |
| Assigned to: | CPU Architecture: | Any | |
[25 Sep 2013 15:25]
MySQL Verification Team
thanks, verified in 5.7 also
[27 Nov 2013 17:49]
Daniel Price
Fixed as of 5.6.16, 5.7.4. Noted in 5.6.16, 5.7.4 changelogs: In "sync0rw.ic", "rw_lock_x_lock_func_nowait" would needlessly call "os_thread_get_curr_id".
[3 Feb 2014 10:48]
Laurynas Biveinis
5.6$ bzr log -r 5655
------------------------------------------------------------
revno: 5655
committer: Aditya A <aditya.a@oracle.com>
branch nick: mysql-5.6
timestamp: Wed 2013-11-27 20:09:55 +0530
message:
Bug#17509710 RW_LOCK_X_LOCK_FUNC_NOWAIT() CALLS
OS_THREAD_GET_CURR_ID() MOSTLY NEEDLESSLY
PROBLEM
-------
os_thread_get_curr_id() was being called for each
invocation of function rw_lock_x_lock_func_nowait(),
but only used when we fail to acquire the locks.
FIX
---
Call os_thread_get_curr_id() only when required.
[Approved by Marko #rb 3967]
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.