Merge lp:~vlad-lesin/percona-server/5.1-gca-bug-1038940-table_cache_speed_up into lp:percona-server/5.1

Proposed by Vlad Lesin on 2013-01-03
Status: Work in progress
Proposed branch: lp:~vlad-lesin/percona-server/5.1-gca-bug-1038940-table_cache_speed_up
Merge into: lp:percona-server/5.1
Diff against target: 441 lines (+185/-99)
10 files modified
Percona-Server/storage/myisam/ha_myisam.cc (+4/-0)
Percona-Server/storage/myisam/mi_close.c (+1/-1)
Percona-Server/storage/myisam/mi_dbug.c (+3/-3)
Percona-Server/storage/myisam/mi_keycache.c (+3/-3)
Percona-Server/storage/myisam/mi_open.c (+67/-13)
Percona-Server/storage/myisam/mi_panic.c (+92/-75)
Percona-Server/storage/myisam/mi_static.c (+1/-1)
Percona-Server/storage/myisam/myisamchk.c (+6/-0)
Percona-Server/storage/myisam/myisamdef.h (+3/-3)
Percona-Server/storage/myisam/myisampack.c (+5/-0)
To merge this branch: bzr merge lp:~vlad-lesin/percona-server/5.1-gca-bug-1038940-table_cache_speed_up
Reviewer Review Type Date Requested Status
Laurynas Biveinis (community) Needs Fixing on 2013-03-25
Stewart Smith 2013-01-03 Pending
Review via email: mp+141711@code.launchpad.net

Description of the change

This MP is port of this http://lists.mysql.com/commits/121507 patch to 5.1(see issue #25137). The main idea of this patch is to replace linked list of opened tables with hash-table to improve performance of open table lookup mechanism.

Jenkins test:
http://jenkins.percona.com/view/PS%205.1/job/percona-server-5.1-param/480

This is just copy of this https://code.launchpad.net/~vlad-lesin/percona-server/5.1-table_cache_speed_up/+merge/120285 MP to 5.1 GCA clone to make merging to 5.5 easier.

To post a comment you must log in.
review: Approve
Stewart Smith (stewart) wrote :

FYI since the 5.5 MP is Work In Progress, I can't yet merge the 5.1 one either.

Vlad Lesin (vlad-lesin) wrote :

Unnecessary changes from mi_panic.c are removed.

The patch is OK, but I'll have to ask you to redo it as a GCA involving all three 5.1/5.5/5.6.

review: Needs Fixing

Unmerged revisions

510. By Vlad Lesin on 2013-01-01

Fix for bug #1038940.

As the number of open tables is increased, table lookup
(testing if a table is already open) and (in particular)
the case when a table is not open, became increasingly more
expensive.

The problem was caused by the open table lookup mechanism,
which was based on traversing a linked list comparing the
file names.

As the list was replaced by a hash table, the lookup
time dropped significantly when used on systems with
a large number of open tables.

The original patch can be found here:
http://lists.mysql.com/commits/121507

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'Percona-Server/storage/myisam/ha_myisam.cc'
2--- Percona-Server/storage/myisam/ha_myisam.cc 2011-11-24 16:33:30 +0000
3+++ Percona-Server/storage/myisam/ha_myisam.cc 2013-01-03 08:27:23 +0000
4@@ -2201,6 +2201,10 @@
5 myisam_hton->create= myisam_create_handler;
6 myisam_hton->panic= myisam_panic;
7 myisam_hton->flags= HTON_CAN_RECREATE | HTON_SUPPORT_LOG_TABLES;
8+
9+ if (mi_init_open_table_hash(table_cache_size))
10+ return 1;
11+
12 return 0;
13 }
14
15
16=== modified file 'Percona-Server/storage/myisam/mi_close.c'
17--- Percona-Server/storage/myisam/mi_close.c 2012-02-15 16:21:38 +0000
18+++ Percona-Server/storage/myisam/mi_close.c 2013-01-03 08:27:23 +0000
19@@ -56,7 +56,7 @@
20 info->opt_flag&= ~(READ_CACHE_USED | WRITE_CACHE_USED);
21 }
22 flag= !--share->reopen;
23- myisam_open_list=list_delete(myisam_open_list,&info->open_list);
24+ my_hash_delete(&myisam_open_table_hash, (uchar *) info);
25 pthread_mutex_unlock(&share->intern_lock);
26
27 my_free(mi_get_rec_buff_ptr(info, info->rec_buff), MYF(MY_ALLOW_ZERO_PTR));
28
29=== modified file 'Percona-Server/storage/myisam/mi_dbug.c'
30--- Percona-Server/storage/myisam/mi_dbug.c 2011-06-30 15:37:13 +0000
31+++ Percona-Server/storage/myisam/mi_dbug.c 2013-01-03 08:27:23 +0000
32@@ -172,13 +172,13 @@
33 my_bool check_table_is_closed(const char *name, const char *where)
34 {
35 char filename[FN_REFLEN];
36- LIST *pos;
37+ uint idx;
38 DBUG_ENTER("check_table_is_closed");
39
40 (void) fn_format(filename,name,"",MI_NAME_IEXT,4+16+32);
41- for (pos=myisam_open_list ; pos ; pos=pos->next)
42+ for (idx= 0; idx < myisam_open_table_hash.records; ++idx)
43 {
44- MI_INFO *info=(MI_INFO*) pos->data;
45+ MI_INFO *info=(MI_INFO*) my_hash_element(&myisam_open_table_hash, idx);
46 MYISAM_SHARE *share=info->s;
47 if (!strcmp(share->unique_file_name,filename))
48 {
49
50=== modified file 'Percona-Server/storage/myisam/mi_keycache.c'
51--- Percona-Server/storage/myisam/mi_keycache.c 2008-03-29 15:56:33 +0000
52+++ Percona-Server/storage/myisam/mi_keycache.c 2013-01-03 08:27:23 +0000
53@@ -137,16 +137,16 @@
54 void mi_change_key_cache(KEY_CACHE *old_key_cache,
55 KEY_CACHE *new_key_cache)
56 {
57- LIST *pos;
58+ uint idx;
59 DBUG_ENTER("mi_change_key_cache");
60
61 /*
62 Lock list to ensure that no one can close the table while we manipulate it
63 */
64 pthread_mutex_lock(&THR_LOCK_myisam);
65- for (pos=myisam_open_list ; pos ; pos=pos->next)
66+ for (idx= 0; idx < myisam_open_table_hash.records; ++idx)
67 {
68- MI_INFO *info= (MI_INFO*) pos->data;
69+ MI_INFO *info=(MI_INFO*) my_hash_element(&myisam_open_table_hash, idx);
70 MYISAM_SHARE *share= info->s;
71 if (share->key_cache == old_key_cache)
72 mi_assign_to_key_cache(info, (ulonglong) ~0, new_key_cache);
73
74=== modified file 'Percona-Server/storage/myisam/mi_open.c'
75--- Percona-Server/storage/myisam/mi_open.c 2011-06-30 15:37:13 +0000
76+++ Percona-Server/storage/myisam/mi_open.c 2013-01-03 08:27:23 +0000
77@@ -46,23 +46,76 @@
78 }
79
80
81-/******************************************************************************
82-** Return the shared struct if the table is already open.
83-** In MySQL the server will handle version issues.
84-******************************************************************************/
85-
86+/*
87+ Get the value used as hash key (helper function for the
88+ open table hash). Function is used as a callback
89+ from the hash table
90+*/
91+static uchar *mi_open_table_hash_key(const uchar *record, size_t *length,
92+ my_bool not_used __attribute__((unused)))
93+{
94+ MI_INFO *info= (MI_INFO *) record;
95+ *length= info->s->unique_name_length;
96+ return (uchar*) info->s->unique_file_name;
97+}
98+
99+/**
100+ Initialize open table hash
101+
102+ Function is normally called from myisam_init
103+ with the system variable table_cache_size used
104+ as hash_size.
105+
106+ @param[in] hash_size Initial has size (elements)
107+ @return inidicates success or failure of initialization
108+ @retval 0 success
109+ @retval 1 failure
110+
111+*/
112+int mi_init_open_table_hash(ulong hash_size)
113+{
114+ if (hash_size == 0)
115+ hash_size= 32; /* default hash size */
116+
117+ if (my_hash_init(&myisam_open_table_hash, &my_charset_filename,
118+ hash_size, 0, 0, mi_open_table_hash_key, 0, 0))
119+ return 1; /* error */
120+
121+ return 0;
122+}
123+
124+
125+/**
126+ Retrieve the shared struct if the table is already
127+ open (i.e in the open table hash)
128+
129+ @param[in] filename table file name
130+ @return shared struct, 0 if not in the cache
131+*/
132 MI_INFO *test_if_reopen(char *filename)
133 {
134- LIST *pos;
135+ HASH_SEARCH_STATE current_record;
136+ int len= strlen(filename);
137
138- for (pos=myisam_open_list ; pos ; pos=pos->next)
139+ MI_INFO *info= (MI_INFO*) my_hash_first(&myisam_open_table_hash,
140+ (uchar *) filename,
141+ len, &current_record);
142+ /*
143+ There might be more than one instance of a table share for
144+ a given table in the hash table. We're interested in the one with
145+ last_version set, so we iterate until we find it
146+ */
147+ while (info)
148 {
149- MI_INFO *info=(MI_INFO*) pos->data;
150 MYISAM_SHARE *share=info->s;
151- if (!strcmp(share->unique_file_name,filename) && share->last_version)
152- return info;
153+ if (share->last_version)
154+ break;
155+
156+ info= (MI_INFO*) my_hash_next(&myisam_open_table_hash,
157+ (uchar *) filename,
158+ len, &current_record);
159 }
160- return 0;
161+ return info;
162 }
163
164
165@@ -650,8 +703,9 @@
166 #ifdef THREAD
167 thr_lock_data_init(&share->lock,&m_info->lock,(void*) m_info);
168 #endif
169- m_info->open_list.data=(void*) m_info;
170- myisam_open_list=list_add(myisam_open_list,&m_info->open_list);
171+
172+ if (my_hash_insert(&myisam_open_table_hash, (uchar *) m_info))
173+ goto err;
174
175 pthread_mutex_unlock(&THR_LOCK_myisam);
176
177
178=== modified file 'Percona-Server/storage/myisam/mi_panic.c'
179--- Percona-Server/storage/myisam/mi_panic.c 2006-12-31 00:32:21 +0000
180+++ Percona-Server/storage/myisam/mi_panic.c 2013-01-03 08:27:23 +0000
181@@ -26,85 +26,102 @@
182 int mi_panic(enum ha_panic_function flag)
183 {
184 int error=0;
185- LIST *list_element,*next_open;
186 MI_INFO *info;
187+ uint idx;
188 DBUG_ENTER("mi_panic");
189
190 pthread_mutex_lock(&THR_LOCK_myisam);
191- for (list_element=myisam_open_list ; list_element ; list_element=next_open)
192- {
193- next_open=list_element->next; /* Save if close */
194- info=(MI_INFO*) list_element->data;
195- switch (flag) {
196- case HA_PANIC_CLOSE:
197- pthread_mutex_unlock(&THR_LOCK_myisam); /* Not exactly right... */
198- if (mi_close(info))
199- error=my_errno;
200- pthread_mutex_lock(&THR_LOCK_myisam);
201- break;
202- case HA_PANIC_WRITE: /* Do this to free databases */
203-#ifdef CANT_OPEN_FILES_TWICE
204- if (info->s->options & HA_OPTION_READ_ONLY_DATA)
205- break;
206-#endif
207- if (flush_key_blocks(info->s->key_cache, info->s->kfile, FLUSH_RELEASE))
208- error=my_errno;
209- if (info->opt_flag & WRITE_CACHE_USED)
210- if (flush_io_cache(&info->rec_cache))
211- error=my_errno;
212- if (info->opt_flag & READ_CACHE_USED)
213- {
214- if (flush_io_cache(&info->rec_cache))
215- error=my_errno;
216- reinit_io_cache(&info->rec_cache,READ_CACHE,0,
217- (pbool) (info->lock_type != F_UNLCK),1);
218- }
219- if (info->lock_type != F_UNLCK && ! info->was_locked)
220- {
221- info->was_locked=info->lock_type;
222- if (mi_lock_database(info,F_UNLCK))
223- error=my_errno;
224- }
225-#ifdef CANT_OPEN_FILES_TWICE
226- if (info->s->kfile >= 0 && my_close(info->s->kfile,MYF(0)))
227- error = my_errno;
228- if (info->dfile >= 0 && my_close(info->dfile,MYF(0)))
229- error = my_errno;
230- info->s->kfile=info->dfile= -1; /* Files aren't open anymore */
231- break;
232-#endif
233- case HA_PANIC_READ: /* Restore to before WRITE */
234-#ifdef CANT_OPEN_FILES_TWICE
235- { /* Open closed files */
236- char name_buff[FN_REFLEN];
237- if (info->s->kfile < 0)
238- if ((info->s->kfile= my_open(fn_format(name_buff,info->filename,"",
239- N_NAME_IEXT,4),info->mode,
240- MYF(MY_WME))) < 0)
241- error = my_errno;
242- if (info->dfile < 0)
243- {
244- if ((info->dfile= my_open(fn_format(name_buff,info->filename,"",
245- N_NAME_DEXT,4),info->mode,
246- MYF(MY_WME))) < 0)
247- error = my_errno;
248- info->rec_cache.file=info->dfile;
249- }
250- }
251-#endif
252- if (info->was_locked)
253- {
254- if (mi_lock_database(info, info->was_locked))
255- error=my_errno;
256- info->was_locked=0;
257- }
258- break;
259- }
260- }
261- if (flag == HA_PANIC_CLOSE)
262- {
263- VOID(mi_log(0)); /* Close log if neaded */
264- ft_free_stopwords();
265+ if (my_hash_inited(&myisam_open_table_hash))
266+ {
267+ if (flag == HA_PANIC_CLOSE)
268+ {
269+ while (myisam_open_table_hash.records)
270+ {
271+ /*
272+ As long as there are records in the hash, fetch the
273+ first, and close it.
274+ */
275+ info=(MI_INFO*) my_hash_element(&myisam_open_table_hash, 0);
276+ pthread_mutex_unlock(&THR_LOCK_myisam); /* Not exactly right... */
277+ if (mi_close(info))
278+ error= my_errno;
279+ pthread_mutex_lock(&THR_LOCK_myisam);
280+ }
281+ (void) mi_log(0); /* Close log if neaded */
282+ ft_free_stopwords();
283+ }
284+ else
285+ {
286+ for (idx= 0; idx < myisam_open_table_hash.records; ++idx)
287+ {
288+ info=(MI_INFO*) my_hash_element(&myisam_open_table_hash, idx);
289+ switch (flag) {
290+ case HA_PANIC_CLOSE: break; /* To eliminate warning. Handled above */
291+ case HA_PANIC_WRITE: /* Do this to free databases */
292+
293+#ifdef CANT_OPEN_FILES_TWICE
294+ if (info->s->options & HA_OPTION_READ_ONLY_DATA)
295+ break;
296+#endif
297+ if (flush_key_blocks(info->s->key_cache, info->s->kfile, FLUSH_RELEASE))
298+ error=my_errno;
299+ if (info->opt_flag & WRITE_CACHE_USED)
300+ if (flush_io_cache(&info->rec_cache))
301+ error=my_errno;
302+ if (info->opt_flag & READ_CACHE_USED)
303+ {
304+ if (flush_io_cache(&info->rec_cache))
305+ error=my_errno;
306+ reinit_io_cache(&info->rec_cache,READ_CACHE,0,
307+ (pbool) (info->lock_type != F_UNLCK),1);
308+ }
309+ if (info->lock_type != F_UNLCK && ! info->was_locked)
310+ {
311+ info->was_locked=info->lock_type;
312+ if (mi_lock_database(info,F_UNLCK))
313+ error=my_errno;
314+ }
315+#ifdef CANT_OPEN_FILES_TWICE
316+ if (info->s->kfile >= 0 && mysql_file_close(info->s->kfile, MYF(0)))
317+ error = my_errno;
318+ if (info->dfile >= 0 && mysql_file_close(info->dfile, MYF(0)))
319+ error = my_errno;
320+ info->s->kfile=info->dfile= -1; /* Files aren't open anymore */
321+ break;
322+#endif
323+ case HA_PANIC_READ: /* Restore to before WRITE */
324+#ifdef CANT_OPEN_FILES_TWICE
325+ { /* Open closed files */
326+ char name_buff[FN_REFLEN];
327+ if (info->s->kfile < 0)
328+ if ((info->s->kfile= mysql_file_open(mi_key_file_kfile,
329+ fn_format(name_buff,
330+ info->filename, "",
331+ N_NAME_IEXT, 4),
332+ info->mode, MYF(MY_WME))) < 0)
333+ error = my_errno;
334+ if (info->dfile < 0)
335+ {
336+ if ((info->dfile= mysql_file_open(mi_key_file_dfile,
337+ fn_format(name_buff,
338+ info->filename, "",
339+ N_NAME_DEXT, 4),
340+ info->mode, MYF(MY_WME))) < 0)
341+ error = my_errno;
342+ info->rec_cache.file=info->dfile;
343+ }
344+ }
345+#endif
346+ if (info->was_locked)
347+ {
348+ if (mi_lock_database(info, info->was_locked))
349+ error=my_errno;
350+ info->was_locked=0;
351+ }
352+ break;
353+ }
354+ }
355+ }
356 }
357 pthread_mutex_unlock(&THR_LOCK_myisam);
358 if (!error)
359
360=== modified file 'Percona-Server/storage/myisam/mi_static.c'
361--- Percona-Server/storage/myisam/mi_static.c 2011-06-30 15:37:13 +0000
362+++ Percona-Server/storage/myisam/mi_static.c 2013-01-03 08:27:23 +0000
363@@ -25,7 +25,7 @@
364 #include "myisamdef.h"
365 #endif
366
367-LIST *myisam_open_list=0;
368+HASH myisam_open_table_hash;
369 uchar NEAR myisam_file_magic[]=
370 { (uchar) 254, (uchar) 254,'\007', '\001', };
371 uchar NEAR myisam_pack_file_magic[]=
372
373=== modified file 'Percona-Server/storage/myisam/myisamchk.c'
374--- Percona-Server/storage/myisam/myisamchk.c 2011-06-30 15:37:13 +0000
375+++ Percona-Server/storage/myisam/myisamchk.c 2013-01-03 08:27:23 +0000
376@@ -96,6 +96,12 @@
377 get_options(&argc,(char***) &argv);
378 myisam_quick_table_bits=decode_bits;
379 error=0;
380+ if (mi_init_open_table_hash(0))
381+ {
382+ fprintf(stderr, "Can't initialize MyISAM storage engine\n");
383+ exit(-1);
384+ }
385+
386 while (--argc >= 0)
387 {
388 int new_error=myisamchk(&check_param, *(argv++));
389
390=== modified file 'Percona-Server/storage/myisam/myisamdef.h'
391--- Percona-Server/storage/myisam/myisamdef.h 2011-11-24 02:01:56 +0000
392+++ Percona-Server/storage/myisam/myisamdef.h 2013-01-03 08:27:23 +0000
393@@ -26,6 +26,7 @@
394 #else
395 #include <my_no_pthread.h>
396 #endif
397+#include "hash.h"
398
399 #if defined(my_write) && !defined(MAP_TO_USE_RAID)
400 #undef my_write /* undef map from my_nosys; We need test-if-disk full */
401@@ -286,7 +287,6 @@
402 uint data_changed; /* Somebody has changed data */
403 uint save_update; /* When using KEY_READ */
404 int save_lastinx;
405- LIST open_list;
406 IO_CACHE rec_cache; /* When cacheing records */
407 uint preload_buff_size; /* When preloading indexes */
408 myf lock_wait; /* is 0 or MY_DONT_WAIT */
409@@ -472,8 +472,7 @@
410 #endif
411
412 /* Some extern variables */
413-
414-extern LIST *myisam_open_list;
415+extern HASH myisam_open_table_hash;
416 extern uchar NEAR myisam_file_magic[],NEAR myisam_pack_file_magic[];
417 extern uint NEAR myisam_read_vec[],NEAR myisam_readnext_vec[];
418 extern uint myisam_quick_table_bits;
419@@ -759,6 +758,7 @@
420 void mi_disable_non_unique_index(MI_INFO *info, ha_rows rows);
421
422 extern MI_INFO *test_if_reopen(char *filename);
423+extern int mi_init_open_table_hash(ulong size);
424 my_bool check_table_is_closed(const char *name, const char *where);
425 int mi_open_datafile(MI_INFO *info, MYISAM_SHARE *share, const char *orn_name,
426 File file_to_dup);
427
428=== modified file 'Percona-Server/storage/myisam/myisampack.c'
429--- Percona-Server/storage/myisam/myisampack.c 2011-06-30 15:37:13 +0000
430+++ Percona-Server/storage/myisam/myisampack.c 2013-01-03 08:27:23 +0000
431@@ -210,6 +210,11 @@
432 MY_INIT(argv[0]);
433
434 load_defaults("my",load_default_groups,&argc,&argv);
435+ if (mi_init_open_table_hash(0))
436+ {
437+ fputs("Can't initialize MyISAM storage engine", stderr);
438+ exit(1);
439+ }
440 default_argv= argv;
441 get_options(&argc,&argv);
442

Subscribers

People subscribed via source and target branches