Hi Sergei, thanks for looking into this patch. Frankly speaking I find it a bit questionable too. Below are links that should answer your questions... What problem do I attempt to solve: https://lists.launchpad.net/maria-developers/msg06118.html How do I attempt to solve it: https://mariadb.atlassian.net/browse/MDEV-4956 32 connections issue simple SELECT against one table. Server has 4 CPU (32 cores + 32 HyperThreads). For every statment we acquire table from table cache and then release table back to the cache. That involves update of 3 lists: unused_tables, per-share used_tables and free_tables. These lists are protected by LOCK_open (see tc_acquire_table() and tc_release_table()). Every time we update global pointer, corresponding cache lines of sibling CPUs have to be invalidated. This is causing expensive memory reads while LOCK_open is held. Oracle solved this problem by partitioning table cache, allowing emulation of something like per-CPU lists. We attempted to split LOCK_open logically, and succeeded at everything but these 3 lists. I attempted lock-free list for free_tables, but TPS rate didn't improve. What we need is to reduce number of these expensive memory reads, and there are two solutions: partition these lists or get rid of them. As we agreed not to partition, I'm trying the latter solution. Why I find this patch questionable? It reduces LOCK_open wait time by 30%, to get close to Oracle wait time, we need to reduce wait time by 90%. We could remove unused_tables as well, but it will be 60% not 90%. Thanks, Sergey On Mon, Sep 09, 2013 at 06:14:13PM +0200, Sergei Golubchik wrote:
Hi, Sergey!
On Aug 27, Sergey Vojtovich wrote:
At lp:maria/10.0
------------------------------------------------------------ revno: 3807 revision-id: svoj@mariadb.org-20130827121233-xh1uyhgfwbhedqyf parent: jplindst@mariadb.org-20130823060357-pww92qxla7o8iir7 committer: Sergey Vojtovich <svoj@mariadb.org> branch nick: 10.0 timestamp: Tue 2013-08-27 16:12:33 +0400 message: MDEV-4956 - Reduce usage of LOCK_open: TABLE_SHARE::tdc.used_tables
- tc_acquire_table and tc_release_table do not access TABLE_SHARE::tdc.used_tables anymore - in tc_acquire_table(): release LOCK_tdc after we relase LOCK_open (saves a few CPU cycles in critical section) - in tc_release_table(): if we reached table cache threshold, evict to-be-released table without moving it to unused_tables. unused_tables must be empty at this point (added assertion).
I don't understand what you're doing here, could you elaborate? E.g. explain in a changeset comment what you've done, why you introduced a new list for tables (all_share), what is its semantics, etc.
Regards, Sergei