Merge lp:~akopytov/percona-server/bug1131187-5.6 into lp:percona-server/5.6
- bug1131187-5.6
- Merge into 5.6
Status: | Merged |
---|---|
Approved by: | Laurynas Biveinis |
Approved revision: | no longer in the source branch. |
Merged at revision: | 354 |
Proposed branch: | lp:~akopytov/percona-server/bug1131187-5.6 |
Merge into: | lp:percona-server/5.6 |
Diff against target: |
424 lines (+113/-73) 7 files modified
Percona-Server/storage/innobase/include/read0read.h (+17/-4) Percona-Server/storage/innobase/include/trx0purge.h (+4/-0) Percona-Server/storage/innobase/include/trx0trx.h (+1/-3) Percona-Server/storage/innobase/read/read0read.cc (+79/-51) Percona-Server/storage/innobase/row/row0sel.cc (+1/-1) Percona-Server/storage/innobase/trx/trx0purge.cc (+7/-2) Percona-Server/storage/innobase/trx/trx0trx.cc (+4/-12) |
To merge this branch: | bzr merge lp:~akopytov/percona-server/bug1131187-5.6 |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Laurynas Biveinis (community) | Approve | ||
Review via email: mp+165452@code.launchpad.net |
Commit message
Description of the change
Manual merge of the fix for bug #1131187 "Remove malloc() from
read_
This patch also incorporates the fix for #1170103 "Memory leak @
read_
GA"
http://
Laurynas Biveinis (laurynas-biveinis) wrote : | # |
Laurynas Biveinis (laurynas-biveinis) : | # |
Alexey Kopytov (akopytov) wrote : | # |
Hi Laurynas,
On Fri, 24 May 2013 09:25:35 -0000, Laurynas Biveinis wrote:
>
> - Upstream read_view_
> trx_ids in a single allocation. After the fix we are back to
> two allocations. It is possible to have a single allocation
> for us too, but I'm not sure whether it's worth it: on one
> hand, it's a single ut_realloc() anyway when trx_ids grows, on
> the other, reallocating the whole view would improve access
> locality and saves one memcpy() in read_view_clone(). Shall we
> open a bug for this or there is no benefit?
>
Yes I was considering that, but didn't like it. That would reduce
flexibility for some further work (and the fact that upstream code
relies on specific layout of data structures in memory is also
problematic). I don't think one extra mempcy() in read_view_clone() will
have any measurable effect.
Preview Diff
1 | === modified file 'Percona-Server/storage/innobase/include/read0read.h' | |||
2 | --- Percona-Server/storage/innobase/include/read0read.h 2013-05-06 15:43:51 +0000 | |||
3 | +++ Percona-Server/storage/innobase/include/read0read.h 2013-05-23 17:46:30 +0000 | |||
4 | @@ -44,8 +44,8 @@ | |||
5 | 44 | /*===============*/ | 44 | /*===============*/ |
6 | 45 | trx_id_t cr_trx_id, /*!< in: trx_id of creating | 45 | trx_id_t cr_trx_id, /*!< in: trx_id of creating |
7 | 46 | transaction, or 0 used in purge */ | 46 | transaction, or 0 used in purge */ |
10 | 47 | mem_heap_t* heap); /*!< in: memory heap from which | 47 | read_view_t*& view); /*!< in,out: pre-allocated view array or |
11 | 48 | allocated */ | 48 | NULL if a new one needs to be created */ |
12 | 49 | /*********************************************************************//** | 49 | /*********************************************************************//** |
13 | 50 | Makes a copy of the oldest existing read view, or opens a new. The view | 50 | Makes a copy of the oldest existing read view, or opens a new. The view |
14 | 51 | must be closed with ..._close. | 51 | must be closed with ..._close. |
15 | @@ -54,8 +54,11 @@ | |||
16 | 54 | read_view_t* | 54 | read_view_t* |
17 | 55 | read_view_purge_open( | 55 | read_view_purge_open( |
18 | 56 | /*=================*/ | 56 | /*=================*/ |
21 | 57 | mem_heap_t* heap); /*!< in: memory heap from which | 57 | read_view_t*& clone_view, /*!< in,out: pre-allocated view that |
22 | 58 | allocated */ | 58 | will be used to clone the oldest view if |
23 | 59 | exists */ | ||
24 | 60 | read_view_t*& view); /*!< in,out: pre-allocated view array or | ||
25 | 61 | NULL if a new one needs to be created */ | ||
26 | 59 | /*********************************************************************//** | 62 | /*********************************************************************//** |
27 | 60 | Remove a read view from the trx_sys->view_list. */ | 63 | Remove a read view from the trx_sys->view_list. */ |
28 | 61 | UNIV_INLINE | 64 | UNIV_INLINE |
29 | @@ -66,6 +69,13 @@ | |||
30 | 66 | bool own_mutex); /*!< in: true if caller owns the | 69 | bool own_mutex); /*!< in: true if caller owns the |
31 | 67 | trx_sys_t::mutex */ | 70 | trx_sys_t::mutex */ |
32 | 68 | /*********************************************************************//** | 71 | /*********************************************************************//** |
33 | 72 | Frees memory allocated by a read view. */ | ||
34 | 73 | UNIV_INTERN | ||
35 | 74 | void | ||
36 | 75 | read_view_free( | ||
37 | 76 | /*===========*/ | ||
38 | 77 | read_view_t*& view); /*< in,out: read view */ | ||
39 | 78 | /*********************************************************************//** | ||
40 | 69 | Closes a consistent read view for MySQL. This function is called at an SQL | 79 | Closes a consistent read view for MySQL. This function is called at an SQL |
41 | 70 | statement end if the trx isolation level is <= TRX_ISO_READ_COMMITTED. */ | 80 | statement end if the trx isolation level is <= TRX_ISO_READ_COMMITTED. */ |
42 | 71 | UNIV_INTERN | 81 | UNIV_INTERN |
43 | @@ -146,6 +156,9 @@ | |||
44 | 146 | this is the "low water mark". */ | 156 | this is the "low water mark". */ |
45 | 147 | ulint n_trx_ids; | 157 | ulint n_trx_ids; |
46 | 148 | /*!< Number of cells in the trx_ids array */ | 158 | /*!< Number of cells in the trx_ids array */ |
47 | 159 | ulint max_trx_ids; | ||
48 | 160 | /*!< Maximum number of cells in the trx_ids | ||
49 | 161 | array */ | ||
50 | 149 | trx_id_t* trx_ids;/*!< Additional trx ids which the read should | 162 | trx_id_t* trx_ids;/*!< Additional trx ids which the read should |
51 | 150 | not see: typically, these are the read-write | 163 | not see: typically, these are the read-write |
52 | 151 | active transactions at the time when the read | 164 | active transactions at the time when the read |
53 | 152 | 165 | ||
54 | === modified file 'Percona-Server/storage/innobase/include/trx0purge.h' | |||
55 | --- Percona-Server/storage/innobase/include/trx0purge.h 2013-05-12 09:13:00 +0000 | |||
56 | +++ Percona-Server/storage/innobase/include/trx0purge.h 2013-05-23 17:46:30 +0000 | |||
57 | @@ -156,6 +156,10 @@ | |||
58 | 156 | parallelized purge operation */ | 156 | parallelized purge operation */ |
59 | 157 | read_view_t* view; /*!< The purge will not remove undo logs | 157 | read_view_t* view; /*!< The purge will not remove undo logs |
60 | 158 | which are >= this view (purge view) */ | 158 | which are >= this view (purge view) */ |
61 | 159 | read_view_t* prebuilt_clone; /*!< Pre-built view which is used as a | ||
62 | 160 | temporary clone of the oldest view in | ||
63 | 161 | read_view_purge_open() */ | ||
64 | 162 | read_view_t* prebuilt_view; /*!< Pre-built view array */ | ||
65 | 159 | volatile ulint n_submitted; /*!< Count of total tasks submitted | 163 | volatile ulint n_submitted; /*!< Count of total tasks submitted |
66 | 160 | to the task queue */ | 164 | to the task queue */ |
67 | 161 | volatile ulint n_completed; /*!< Count of total tasks completed */ | 165 | volatile ulint n_completed; /*!< Count of total tasks completed */ |
68 | 162 | 166 | ||
69 | === modified file 'Percona-Server/storage/innobase/include/trx0trx.h' | |||
70 | --- Percona-Server/storage/innobase/include/trx0trx.h 2013-05-12 06:24:46 +0000 | |||
71 | +++ Percona-Server/storage/innobase/include/trx0trx.h 2013-05-23 17:46:30 +0000 | |||
72 | @@ -911,9 +911,6 @@ | |||
73 | 911 | survive over a transaction commit, if | 911 | survive over a transaction commit, if |
74 | 912 | it is a stored procedure with a COMMIT | 912 | it is a stored procedure with a COMMIT |
75 | 913 | WORK statement, for instance */ | 913 | WORK statement, for instance */ |
76 | 914 | mem_heap_t* global_read_view_heap; | ||
77 | 915 | /*!< memory heap for the global read | ||
78 | 916 | view */ | ||
79 | 917 | read_view_t* global_read_view; | 914 | read_view_t* global_read_view; |
80 | 918 | /*!< consistent read view associated | 915 | /*!< consistent read view associated |
81 | 919 | to a transaction or NULL */ | 916 | to a transaction or NULL */ |
82 | @@ -923,6 +920,7 @@ | |||
83 | 923 | associated to a transaction (i.e. | 920 | associated to a transaction (i.e. |
84 | 924 | same as global_read_view) or read view | 921 | same as global_read_view) or read view |
85 | 925 | associated to a cursor */ | 922 | associated to a cursor */ |
86 | 923 | read_view_t* prebuilt_view; /* pre-built view array */ | ||
87 | 926 | /*------------------------------*/ | 924 | /*------------------------------*/ |
88 | 927 | UT_LIST_BASE_NODE_T(trx_named_savept_t) | 925 | UT_LIST_BASE_NODE_T(trx_named_savept_t) |
89 | 928 | trx_savepoints; /*!< savepoints set with SAVEPOINT ..., | 926 | trx_savepoints; /*!< savepoints set with SAVEPOINT ..., |
90 | 929 | 927 | ||
91 | === modified file 'Percona-Server/storage/innobase/read/read0read.cc' | |||
92 | --- Percona-Server/storage/innobase/read/read0read.cc 2013-05-06 15:43:51 +0000 | |||
93 | +++ Percona-Server/storage/innobase/read/read0read.cc 2013-05-23 17:46:30 +0000 | |||
94 | @@ -183,16 +183,29 @@ | |||
95 | 183 | read_view_create_low( | 183 | read_view_create_low( |
96 | 184 | /*=================*/ | 184 | /*=================*/ |
97 | 185 | ulint n, /*!< in: number of cells in the trx_ids array */ | 185 | ulint n, /*!< in: number of cells in the trx_ids array */ |
99 | 186 | mem_heap_t* heap) /*!< in: memory heap from which allocated */ | 186 | read_view_t*& view) /*!< in,out: pre-allocated view array or NULL if |
100 | 187 | a new one needs to be created */ | ||
101 | 187 | { | 188 | { |
107 | 188 | read_view_t* view; | 189 | if (view == NULL) { |
108 | 189 | 190 | view = static_cast<read_view_t*>( | |
109 | 190 | view = static_cast<read_view_t*>( | 191 | ut_malloc(sizeof(read_view_t))); |
110 | 191 | mem_heap_alloc( | 192 | view->max_trx_ids = 0; |
111 | 192 | heap, sizeof(*view) + n * sizeof(*view->trx_ids))); | 193 | view->trx_ids = NULL; |
112 | 194 | } | ||
113 | 195 | |||
114 | 196 | if (UNIV_UNLIKELY(view->max_trx_ids < n)) { | ||
115 | 197 | |||
116 | 198 | /* avoid frequent reallocations by extending the array to the | ||
117 | 199 | desired size + 10% */ | ||
118 | 200 | |||
119 | 201 | view->max_trx_ids = n + n / 10; | ||
120 | 202 | view->trx_ids = static_cast<trx_id_t*>( | ||
121 | 203 | ut_realloc(view->trx_ids, | ||
122 | 204 | view->max_trx_ids * | ||
123 | 205 | sizeof *view->trx_ids)); | ||
124 | 206 | } | ||
125 | 193 | 207 | ||
126 | 194 | view->n_trx_ids = n; | 208 | view->n_trx_ids = n; |
127 | 195 | view->trx_ids = (trx_id_t*) &view[1]; | ||
128 | 196 | 209 | ||
129 | 197 | return(view); | 210 | return(view); |
130 | 198 | } | 211 | } |
131 | @@ -207,37 +220,30 @@ | |||
132 | 207 | read_view_t* | 220 | read_view_t* |
133 | 208 | read_view_clone( | 221 | read_view_clone( |
134 | 209 | /*============*/ | 222 | /*============*/ |
138 | 210 | const read_view_t* view, /*!< in: view to clone */ | 223 | const read_view_t* view, /*!< in: view to clone */ |
139 | 211 | mem_heap_t* heap) /*!< in: memory heap | 224 | read_view_t*& prebuilt_clone) /*!< in,out: prebuilt view or |
140 | 212 | from which allocated */ | 225 | NULL */ |
141 | 213 | { | 226 | { |
142 | 214 | ulint sz; | ||
143 | 215 | read_view_t* clone; | 227 | read_view_t* clone; |
145 | 216 | read_view_t* new_view; | 228 | trx_id_t* old_trx_ids; |
146 | 229 | ulint old_max_trx_ids; | ||
147 | 217 | 230 | ||
148 | 218 | ut_ad(mutex_own(&trx_sys->mutex)); | 231 | ut_ad(mutex_own(&trx_sys->mutex)); |
149 | 219 | 232 | ||
171 | 220 | /* Allocate space for two views. */ | 233 | clone = read_view_create_low(view->n_trx_ids, prebuilt_clone); |
172 | 221 | 234 | ||
173 | 222 | sz = sizeof(*view) + view->n_trx_ids * sizeof(*view->trx_ids); | 235 | old_trx_ids = clone->trx_ids; |
174 | 223 | 236 | old_max_trx_ids = clone->max_trx_ids; | |
175 | 224 | /* Add an extra trx_id_t slot for the new view. */ | 237 | |
176 | 225 | 238 | memcpy(clone, view, sizeof(*view)); | |
177 | 226 | clone = static_cast<read_view_t*>( | 239 | |
178 | 227 | mem_heap_alloc(heap, (sz * 2) + sizeof(trx_id_t))); | 240 | clone->trx_ids = old_trx_ids; |
179 | 228 | 241 | clone->max_trx_ids = old_max_trx_ids; | |
180 | 229 | /* Only the contents of the old view are important, the new view | 242 | |
181 | 230 | will be created from this and so we don't copy that across. */ | 243 | if (view->n_trx_ids) { |
182 | 231 | 244 | memcpy(clone->trx_ids, view->trx_ids, | |
183 | 232 | memcpy(clone, view, sz); | 245 | view->n_trx_ids * sizeof(trx_id_t)); |
184 | 233 | 246 | } | |
164 | 234 | clone->trx_ids = (trx_id_t*) &clone[1]; | ||
165 | 235 | |||
166 | 236 | new_view = (read_view_t*) &clone->trx_ids[clone->n_trx_ids]; | ||
167 | 237 | new_view->trx_ids = (trx_id_t*) &new_view[1]; | ||
168 | 238 | new_view->n_trx_ids = clone->n_trx_ids + 1; | ||
169 | 239 | |||
170 | 240 | ut_a(new_view->n_trx_ids == view->n_trx_ids + 1); | ||
185 | 241 | 247 | ||
186 | 242 | return(clone); | 248 | return(clone); |
187 | 243 | } | 249 | } |
188 | @@ -331,15 +337,14 @@ | |||
189 | 331 | /*===================*/ | 337 | /*===================*/ |
190 | 332 | trx_id_t cr_trx_id, /*!< in: trx_id of creating | 338 | trx_id_t cr_trx_id, /*!< in: trx_id of creating |
191 | 333 | transaction, or 0 used in purge */ | 339 | transaction, or 0 used in purge */ |
194 | 334 | mem_heap_t* heap) /*!< in: memory heap from which | 340 | read_view_t*& view) /*!< in,out: pre-allocated view array or |
195 | 335 | allocated */ | 341 | NULL if a new one needs to be created */ |
196 | 336 | { | 342 | { |
197 | 337 | read_view_t* view; | ||
198 | 338 | ulint n_trx = UT_LIST_GET_LEN(trx_sys->rw_trx_list); | 343 | ulint n_trx = UT_LIST_GET_LEN(trx_sys->rw_trx_list); |
199 | 339 | 344 | ||
200 | 340 | ut_ad(mutex_own(&trx_sys->mutex)); | 345 | ut_ad(mutex_own(&trx_sys->mutex)); |
201 | 341 | 346 | ||
203 | 342 | view = read_view_create_low(n_trx, heap); | 347 | view = read_view_create_low(n_trx, view); |
204 | 343 | 348 | ||
205 | 344 | view->undo_no = 0; | 349 | view->undo_no = 0; |
206 | 345 | view->type = VIEW_NORMAL; | 350 | view->type = VIEW_NORMAL; |
207 | @@ -379,14 +384,12 @@ | |||
208 | 379 | /*===============*/ | 384 | /*===============*/ |
209 | 380 | trx_id_t cr_trx_id, /*!< in: trx_id of creating | 385 | trx_id_t cr_trx_id, /*!< in: trx_id of creating |
210 | 381 | transaction, or 0 used in purge */ | 386 | transaction, or 0 used in purge */ |
213 | 382 | mem_heap_t* heap) /*!< in: memory heap from which | 387 | read_view_t*& view) /*!< in,out: pre-allocated view array or |
214 | 383 | allocated */ | 388 | NULL if a new one needs to be created */ |
215 | 384 | { | 389 | { |
216 | 385 | read_view_t* view; | ||
217 | 386 | |||
218 | 387 | mutex_enter(&trx_sys->mutex); | 390 | mutex_enter(&trx_sys->mutex); |
219 | 388 | 391 | ||
221 | 389 | view = read_view_open_now_low(cr_trx_id, heap); | 392 | view = read_view_open_now_low(cr_trx_id, view); |
222 | 390 | 393 | ||
223 | 391 | mutex_exit(&trx_sys->mutex); | 394 | mutex_exit(&trx_sys->mutex); |
224 | 392 | 395 | ||
225 | @@ -403,8 +406,11 @@ | |||
226 | 403 | read_view_t* | 406 | read_view_t* |
227 | 404 | read_view_purge_open( | 407 | read_view_purge_open( |
228 | 405 | /*=================*/ | 408 | /*=================*/ |
231 | 406 | mem_heap_t* heap) /*!< in: memory heap from which | 409 | read_view_t*& prebuilt_clone, /*!< in,out: pre-allocated view that |
232 | 407 | allocated */ | 410 | will be used to clone the oldest view if |
233 | 411 | exists */ | ||
234 | 412 | read_view_t*& prebuilt_view) /*!< in,out: pre-allocated view array or | ||
235 | 413 | NULL if a new one needs to be created */ | ||
236 | 408 | { | 414 | { |
237 | 409 | ulint i; | 415 | ulint i; |
238 | 410 | read_view_t* view; | 416 | read_view_t* view; |
239 | @@ -418,16 +424,16 @@ | |||
240 | 418 | 424 | ||
241 | 419 | if (oldest_view == NULL) { | 425 | if (oldest_view == NULL) { |
242 | 420 | 426 | ||
244 | 421 | view = read_view_open_now_low(0, heap); | 427 | view = read_view_open_now_low(0, prebuilt_view); |
245 | 422 | 428 | ||
246 | 423 | mutex_exit(&trx_sys->mutex); | 429 | mutex_exit(&trx_sys->mutex); |
247 | 424 | 430 | ||
248 | 425 | return(view); | 431 | return(view); |
249 | 426 | } | 432 | } |
250 | 427 | 433 | ||
252 | 428 | /* Allocate space for both views, the oldest and the new purge view. */ | 434 | /* Clone the oldest view to a pre-allocated clone view */ |
253 | 429 | 435 | ||
255 | 430 | oldest_view = read_view_clone(oldest_view, heap); | 436 | oldest_view = read_view_clone(oldest_view, prebuilt_clone); |
256 | 431 | 437 | ||
257 | 432 | ut_ad(read_view_validate(oldest_view)); | 438 | ut_ad(read_view_validate(oldest_view)); |
258 | 433 | 439 | ||
259 | @@ -436,7 +442,7 @@ | |||
260 | 436 | ut_a(oldest_view->creator_trx_id > 0); | 442 | ut_a(oldest_view->creator_trx_id > 0); |
261 | 437 | creator_trx_id = oldest_view->creator_trx_id; | 443 | creator_trx_id = oldest_view->creator_trx_id; |
262 | 438 | 444 | ||
264 | 439 | view = (read_view_t*) &oldest_view->trx_ids[oldest_view->n_trx_ids]; | 445 | view = read_view_create_low(oldest_view->n_trx_ids + 1, prebuilt_view); |
265 | 440 | 446 | ||
266 | 441 | /* Add the creator transaction id in the trx_ids array in the | 447 | /* Add the creator transaction id in the trx_ids array in the |
267 | 442 | correct slot. */ | 448 | correct slot. */ |
268 | @@ -490,8 +496,6 @@ | |||
269 | 490 | 496 | ||
270 | 491 | read_view_remove(trx->global_read_view, false); | 497 | read_view_remove(trx->global_read_view, false); |
271 | 492 | 498 | ||
272 | 493 | mem_heap_empty(trx->global_read_view_heap); | ||
273 | 494 | |||
274 | 495 | trx->read_view = NULL; | 499 | trx->read_view = NULL; |
275 | 496 | trx->global_read_view = NULL; | 500 | trx->global_read_view = NULL; |
276 | 497 | } | 501 | } |
277 | @@ -566,6 +570,28 @@ | |||
278 | 566 | } | 570 | } |
279 | 567 | 571 | ||
280 | 568 | /*********************************************************************//** | 572 | /*********************************************************************//** |
281 | 573 | Frees resource allocated by a read view. */ | ||
282 | 574 | UNIV_INTERN | ||
283 | 575 | void | ||
284 | 576 | read_view_free( | ||
285 | 577 | /*===========*/ | ||
286 | 578 | read_view_t*& view) /*< in,out: read view */ | ||
287 | 579 | { | ||
288 | 580 | if (view == NULL) { | ||
289 | 581 | |||
290 | 582 | return; | ||
291 | 583 | } | ||
292 | 584 | |||
293 | 585 | if (view->trx_ids != NULL) { | ||
294 | 586 | ut_free(view->trx_ids); | ||
295 | 587 | } | ||
296 | 588 | |||
297 | 589 | ut_free(view); | ||
298 | 590 | |||
299 | 591 | view = NULL; | ||
300 | 592 | } | ||
301 | 593 | |||
302 | 594 | /*********************************************************************//** | ||
303 | 569 | Create a high-granularity consistent cursor view for mysql to be used | 595 | Create a high-granularity consistent cursor view for mysql to be used |
304 | 570 | in cursors. In this consistent read view modifications done by the | 596 | in cursors. In this consistent read view modifications done by the |
305 | 571 | creating transaction after the cursor is created or future transactions | 597 | creating transaction after the cursor is created or future transactions |
306 | @@ -601,7 +627,8 @@ | |||
307 | 601 | 627 | ||
308 | 602 | n_trx = UT_LIST_GET_LEN(trx_sys->rw_trx_list); | 628 | n_trx = UT_LIST_GET_LEN(trx_sys->rw_trx_list); |
309 | 603 | 629 | ||
311 | 604 | curview->read_view = read_view_create_low(n_trx, curview->heap); | 630 | curview->read_view = NULL; |
312 | 631 | read_view_create_low(n_trx, curview->read_view); | ||
313 | 605 | 632 | ||
314 | 606 | view = curview->read_view; | 633 | view = curview->read_view; |
315 | 607 | view->undo_no = cr_trx->undo_no; | 634 | view->undo_no = cr_trx->undo_no; |
316 | @@ -653,6 +680,7 @@ | |||
317 | 653 | trx->n_mysql_tables_in_use += curview->n_mysql_tables_in_use; | 680 | trx->n_mysql_tables_in_use += curview->n_mysql_tables_in_use; |
318 | 654 | 681 | ||
319 | 655 | read_view_remove(curview->read_view, false); | 682 | read_view_remove(curview->read_view, false); |
320 | 683 | read_view_free(curview->read_view); | ||
321 | 656 | 684 | ||
322 | 657 | trx->read_view = trx->global_read_view; | 685 | trx->read_view = trx->global_read_view; |
323 | 658 | 686 | ||
324 | 659 | 687 | ||
325 | === modified file 'Percona-Server/storage/innobase/row/row0sel.cc' | |||
326 | --- Percona-Server/storage/innobase/row/row0sel.cc 2013-05-13 04:25:56 +0000 | |||
327 | +++ Percona-Server/storage/innobase/row/row0sel.cc 2013-05-23 17:46:30 +0000 | |||
328 | @@ -5206,7 +5206,7 @@ | |||
329 | 5206 | && !trx->read_view) { | 5206 | && !trx->read_view) { |
330 | 5207 | 5207 | ||
331 | 5208 | trx->read_view = read_view_open_now( | 5208 | trx->read_view = read_view_open_now( |
333 | 5209 | trx->id, trx->global_read_view_heap); | 5209 | trx->id, trx->prebuilt_view); |
334 | 5210 | 5210 | ||
335 | 5211 | trx->global_read_view = trx->read_view; | 5211 | trx->global_read_view = trx->read_view; |
336 | 5212 | } | 5212 | } |
337 | 5213 | 5213 | ||
338 | === modified file 'Percona-Server/storage/innobase/trx/trx0purge.cc' | |||
339 | --- Percona-Server/storage/innobase/trx/trx0purge.cc 2013-05-12 09:13:00 +0000 | |||
340 | +++ Percona-Server/storage/innobase/trx/trx0purge.cc 2013-05-23 17:46:30 +0000 | |||
341 | @@ -152,7 +152,8 @@ | |||
342 | 152 | purge_sys->query = trx_purge_graph_build( | 152 | purge_sys->query = trx_purge_graph_build( |
343 | 153 | purge_sys->trx, n_purge_threads); | 153 | purge_sys->trx, n_purge_threads); |
344 | 154 | 154 | ||
346 | 155 | purge_sys->view = read_view_purge_open(purge_sys->heap); | 155 | purge_sys->view = read_view_purge_open(purge_sys->prebuilt_clone, |
347 | 156 | purge_sys->prebuilt_view); | ||
348 | 156 | } | 157 | } |
349 | 157 | 158 | ||
350 | 158 | /************************************************************************ | 159 | /************************************************************************ |
351 | @@ -173,6 +174,9 @@ | |||
352 | 173 | 174 | ||
353 | 174 | purge_sys->sess = NULL; | 175 | purge_sys->sess = NULL; |
354 | 175 | 176 | ||
355 | 177 | read_view_free(purge_sys->prebuilt_view); | ||
356 | 178 | read_view_free(purge_sys->prebuilt_clone); | ||
357 | 179 | |||
358 | 176 | purge_sys->view = NULL; | 180 | purge_sys->view = NULL; |
359 | 177 | 181 | ||
360 | 178 | rw_lock_free(&purge_sys->latch); | 182 | rw_lock_free(&purge_sys->latch); |
361 | @@ -1200,7 +1204,8 @@ | |||
362 | 1200 | 1204 | ||
363 | 1201 | mem_heap_empty(purge_sys->heap); | 1205 | mem_heap_empty(purge_sys->heap); |
364 | 1202 | 1206 | ||
366 | 1203 | purge_sys->view = read_view_purge_open(purge_sys->heap); | 1207 | purge_sys->view = read_view_purge_open(purge_sys->prebuilt_clone, |
367 | 1208 | purge_sys->prebuilt_view); | ||
368 | 1204 | 1209 | ||
369 | 1205 | rw_lock_x_unlock(&purge_sys->latch); | 1210 | rw_lock_x_unlock(&purge_sys->latch); |
370 | 1206 | 1211 | ||
371 | 1207 | 1212 | ||
372 | === modified file 'Percona-Server/storage/innobase/trx/trx0trx.cc' | |||
373 | --- Percona-Server/storage/innobase/trx/trx0trx.cc 2013-05-22 08:57:37 +0000 | |||
374 | +++ Percona-Server/storage/innobase/trx/trx0trx.cc 2013-05-23 17:46:30 +0000 | |||
375 | @@ -132,8 +132,6 @@ | |||
376 | 132 | 132 | ||
377 | 133 | trx->search_latch_timeout = BTR_SEA_TIMEOUT; | 133 | trx->search_latch_timeout = BTR_SEA_TIMEOUT; |
378 | 134 | 134 | ||
379 | 135 | trx->global_read_view_heap = mem_heap_create(256); | ||
380 | 136 | |||
381 | 137 | trx->io_reads = 0; | 135 | trx->io_reads = 0; |
382 | 138 | trx->io_read = 0; | 136 | trx->io_read = 0; |
383 | 139 | trx->io_reads_wait_timer = 0; | 137 | trx->io_reads_wait_timer = 0; |
384 | @@ -240,10 +238,6 @@ | |||
385 | 240 | 238 | ||
386 | 241 | ut_a(UT_LIST_GET_LEN(trx->lock.trx_locks) == 0); | 239 | ut_a(UT_LIST_GET_LEN(trx->lock.trx_locks) == 0); |
387 | 242 | 240 | ||
388 | 243 | if (trx->global_read_view_heap) { | ||
389 | 244 | mem_heap_free(trx->global_read_view_heap); | ||
390 | 245 | } | ||
391 | 246 | |||
392 | 247 | ut_a(ib_vector_is_empty(trx->autoinc_locks)); | 241 | ut_a(ib_vector_is_empty(trx->autoinc_locks)); |
393 | 248 | /* We allocated a dedicated heap for the vector. */ | 242 | /* We allocated a dedicated heap for the vector. */ |
394 | 249 | ib_vector_free(trx->autoinc_locks); | 243 | ib_vector_free(trx->autoinc_locks); |
395 | @@ -255,6 +249,8 @@ | |||
396 | 255 | 249 | ||
397 | 256 | mutex_free(&trx->mutex); | 250 | mutex_free(&trx->mutex); |
398 | 257 | 251 | ||
399 | 252 | read_view_free(trx->prebuilt_view); | ||
400 | 253 | |||
401 | 258 | mem_free(trx); | 254 | mem_free(trx); |
402 | 259 | } | 255 | } |
403 | 260 | 256 | ||
404 | @@ -1130,8 +1126,6 @@ | |||
405 | 1130 | 1126 | ||
406 | 1131 | if (trx->global_read_view != NULL) { | 1127 | if (trx->global_read_view != NULL) { |
407 | 1132 | 1128 | ||
408 | 1133 | mem_heap_empty(trx->global_read_view_heap); | ||
409 | 1134 | |||
410 | 1135 | trx->global_read_view = NULL; | 1129 | trx->global_read_view = NULL; |
411 | 1136 | } | 1130 | } |
412 | 1137 | 1131 | ||
413 | @@ -1383,10 +1377,8 @@ | |||
414 | 1383 | } | 1377 | } |
415 | 1384 | 1378 | ||
416 | 1385 | if (!trx->read_view) { | 1379 | if (!trx->read_view) { |
421 | 1386 | 1380 | trx->read_view = read_view_open_now(trx->id, | |
422 | 1387 | trx->read_view = read_view_open_now( | 1381 | trx->prebuilt_view); |
419 | 1388 | trx->id, trx->global_read_view_heap); | ||
420 | 1389 | |||
423 | 1390 | trx->global_read_view = trx->read_view; | 1382 | trx->global_read_view = trx->read_view; |
424 | 1391 | } | 1383 | } |
425 | 1392 | 1384 |
Everything is OK. Just one follow-up question:
- Upstream read_view_ create_ low() allocates the read view and its
trx_ids in a single allocation. After the fix we are back to
two allocations. It is possible to have a single allocation
for us too, but I'm not sure whether it's worth it: on one
hand, it's a single ut_realloc() anyway when trx_ids grows, on
the other, reallocating the whole view would improve access
locality and saves one memcpy() in read_view_clone(). Shall we
open a bug for this or there is no benefit?