PBXT

Merge lp:~vkolesnikov/pbxt/pbxt-mmap-direct-read-deadlock into lp:pbxt

  1. pbxt-mmap-direct-read-deadlock
  2. Merge into trunk
Proposed by Vladimir Kolesnikov
Status: Merged
Merged at revision: not available
Proposed branch: lp:~vkolesnikov/pbxt/pbxt-mmap-direct-read-deadlock
Merge into: lp:pbxt
Diff against target: None lines
To merge this branch: bzr merge lp:~vkolesnikov/pbxt/pbxt-mmap-direct-read-deadlock
Reviewer Review Type Date Requested Status
PBXT Core Pending
Review via email: mp+4112@code.launchpad.net
To post a comment you must log in.
lp:~vkolesnikov/pbxt/pbxt-mmap-direct-read-deadlock updated
565. By Vladimir Kolesnikov

changed amotic operations code to use XADD

566. By Vladimir Kolesnikov

minor fixes to locking

567. By Vladimir Kolesnikov

removed wrong assertion

Revision history for this message
Paul McCullagh (paul-mccullagh) wrote :
Download full text (3.5 KiB)

Hi Vlad,

This implementation can be optimized a bit.

In particular, we should not always lock and copy the record header
again after every call to tab_visible().

This is only required in the case that we continue to load the record
after the switch (tab_vis) statement.

This saves time in all cases where the tab_visible() call does not
return a valid record.

On Mar 3, 2009, at 8:10 PM, Vladimir Kolesnikov wrote:

> Vladimir Kolesnikov has proposed merging lp:~vkolesnikov/pbxt/pbxt-
> mmap-direct-read-deadlock into lp:pbxt.
>
> Requested reviews:
> PBXT Core (pbxt-core)
> --
> https://code.launchpad.net/~vkolesnikov/pbxt/pbxt-mmap-direct-read-
> deadlock/+merge/4112
> Your team PBXT Core is subscribed to branch lp:pbxt.
> === modified file 'src/table_xt.cc'
> --- src/table_xt.cc 2009-02-27 09:22:44 +0000
> +++ src/table_xt.cc 2009-03-03 19:02:32 +0000
> @@ -4868,6 +4868,8 @@
> xtRecordID new_rec_id;
> xtBool ptr_locked;
> xtRecordID invalid_rec = 0;
> + XTTabRecHeadDRec rec_head;
> + int tab_vis;
>
> next_page:
> if (!ot->ot_on_page) {
> @@ -4900,6 +4902,9 @@
> XT_LOCK_MEMORY_PTR(buff_ptr, ot->ot_rec_file,
> xt_rec_id_to_rec_offset(tab, ot->ot_seq_rec_id), tab-
> >tab_rows.tci_page_size, &ot->ot_thread->st_statistics.st_rec, ot-
> >ot_thread);
> if (!buff_ptr)
> return FAILED;
> + memcpy(&rec_head, buff_ptr, sizeof(XTTabRecHeadDRec));
> + XT_UNLOCK_MEMORY_PTR(ot->ot_rec_file, ot->ot_thread);
> + buff_ptr = (xtWord1 *)&rec_head;
> }
>
> /* This is the current record: */
> @@ -4911,20 +4916,29 @@
> ot->ot_seq_offset += rec_size;
>
> retry:
> - switch (tab_visible(ot, (XTTabRecHeadDPtr) buff_ptr, &new_rec_id)) {
> + tab_vis = tab_visible(ot, (XTTabRecHeadDPtr) buff_ptr, &new_rec_id);
> + if (ptr_locked) {
> + XT_LOCK_MEMORY_PTR(buff_ptr, ot->ot_rec_file,
> xt_rec_id_to_rec_offset(tab, ot->ot_seq_rec_id - 1),
> + tab->tab_rows.tci_page_size, &ot->ot_thread-
> >st_statistics.st_rec, ot->ot_thread);
> +
> + if (!buff_ptr)
> + return FAILED;
> +
> + if (memcmp(&rec_head, buff_ptr, sizeof(XTTabRecHeadDRec))) {
> + memcpy(&rec_head, buff_ptr, sizeof(XTTabRecHeadDRec));
> + XT_UNLOCK_MEMORY_PTR(ot->ot_rec_file, ot->ot_thread);
> + goto retry;
> + }
> +
> + XT_UNLOCK_MEMORY_PTR(ot->ot_rec_file, ot->ot_thread);
> + }
> +
> + switch (tab_vis) {
> case FALSE:
> - if (ptr_locked) {
> - XT_UNLOCK_MEMORY_PTR(ot->ot_rec_file, ot->ot_thread);
> - ptr_locked = FALSE;
> - }
> goto next_record;
> case XT_ERR:
> goto failed;
> case XT_NEW:
> - if (ptr_locked) {
> - XT_UNLOCK_MEMORY_PTR(ot->ot_rec_file, ot->ot_thread);
> - ptr_locked = FALSE;
> - }
> buff_ptr = ot->ot_row_rbuffer;
> if (!xt_tab_get_rec_data(ot, new_rec_id, rec_size, ot-
> >ot_row_rbuffer))
> return XT_ERR;
> @@ -4937,12 +4951,6 @@
> /* Don't re-read for the same record twice: */
> invalid_rec = ot->ot_curr_rec_id;
>
> - /* Unlock the memory map: */
> - if (ptr_locked) {
> - XT_UNLOCK_MEMORY_PTR(ot->ot_rec_file, ot->ot_thread);
> - ptr_locked = FALSE;
> - }
> -
> /* Undo move to next: */
> ot->ot_seq_rec_id--;
> ot->ot_seq_offset -= rec_size;
> @@ -49...

Read more...

lp:~vkolesnikov/pbxt/pbxt-mmap-direct-read-deadlock updated
568. By Vladimir Kolesnikov

removed unnecessary checking of record header change after tab_visible

569. By Vladimir Kolesnikov

fixed a bug in the new locking code - when searching for a row lock, lock ranges were not taken into account

570. By Vladimir Kolesnikov

fixed asm atomic operations code for windows

571. By Vladimir Kolesnikov

merge from upstream

572. By Vladimir Kolesnikov

changed linux version of atomic operations to use lock/add instead of xadd

573. By Paul McCullagh

Different logic in the item not found case

574. By Vladimir Kolesnikov

another fix to row-locking

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/table_xt.cc'
2--- src/table_xt.cc 2009-02-27 09:22:44 +0000
3+++ src/table_xt.cc 2009-03-03 19:02:32 +0000
4@@ -4868,6 +4868,8 @@
5 xtRecordID new_rec_id;
6 xtBool ptr_locked;
7 xtRecordID invalid_rec = 0;
8+ XTTabRecHeadDRec rec_head;
9+ int tab_vis;
10
11 next_page:
12 if (!ot->ot_on_page) {
13@@ -4900,6 +4902,9 @@
14 XT_LOCK_MEMORY_PTR(buff_ptr, ot->ot_rec_file, xt_rec_id_to_rec_offset(tab, ot->ot_seq_rec_id), tab->tab_rows.tci_page_size, &ot->ot_thread->st_statistics.st_rec, ot->ot_thread);
15 if (!buff_ptr)
16 return FAILED;
17+ memcpy(&rec_head, buff_ptr, sizeof(XTTabRecHeadDRec));
18+ XT_UNLOCK_MEMORY_PTR(ot->ot_rec_file, ot->ot_thread);
19+ buff_ptr = (xtWord1 *)&rec_head;
20 }
21
22 /* This is the current record: */
23@@ -4911,20 +4916,29 @@
24 ot->ot_seq_offset += rec_size;
25
26 retry:
27- switch (tab_visible(ot, (XTTabRecHeadDPtr) buff_ptr, &new_rec_id)) {
28+ tab_vis = tab_visible(ot, (XTTabRecHeadDPtr) buff_ptr, &new_rec_id);
29+ if (ptr_locked) {
30+ XT_LOCK_MEMORY_PTR(buff_ptr, ot->ot_rec_file, xt_rec_id_to_rec_offset(tab, ot->ot_seq_rec_id - 1),
31+ tab->tab_rows.tci_page_size, &ot->ot_thread->st_statistics.st_rec, ot->ot_thread);
32+
33+ if (!buff_ptr)
34+ return FAILED;
35+
36+ if (memcmp(&rec_head, buff_ptr, sizeof(XTTabRecHeadDRec))) {
37+ memcpy(&rec_head, buff_ptr, sizeof(XTTabRecHeadDRec));
38+ XT_UNLOCK_MEMORY_PTR(ot->ot_rec_file, ot->ot_thread);
39+ goto retry;
40+ }
41+
42+ XT_UNLOCK_MEMORY_PTR(ot->ot_rec_file, ot->ot_thread);
43+ }
44+
45+ switch (tab_vis) {
46 case FALSE:
47- if (ptr_locked) {
48- XT_UNLOCK_MEMORY_PTR(ot->ot_rec_file, ot->ot_thread);
49- ptr_locked = FALSE;
50- }
51 goto next_record;
52 case XT_ERR:
53 goto failed;
54 case XT_NEW:
55- if (ptr_locked) {
56- XT_UNLOCK_MEMORY_PTR(ot->ot_rec_file, ot->ot_thread);
57- ptr_locked = FALSE;
58- }
59 buff_ptr = ot->ot_row_rbuffer;
60 if (!xt_tab_get_rec_data(ot, new_rec_id, rec_size, ot->ot_row_rbuffer))
61 return XT_ERR;
62@@ -4937,12 +4951,6 @@
63 /* Don't re-read for the same record twice: */
64 invalid_rec = ot->ot_curr_rec_id;
65
66- /* Unlock the memory map: */
67- if (ptr_locked) {
68- XT_UNLOCK_MEMORY_PTR(ot->ot_rec_file, ot->ot_thread);
69- ptr_locked = FALSE;
70- }
71-
72 /* Undo move to next: */
73 ot->ot_seq_rec_id--;
74 ot->ot_seq_offset -= rec_size;
75@@ -4992,15 +5000,11 @@
76 break;
77 }
78 }
79- if (ptr_locked)
80- XT_UNLOCK_MEMORY_PTR(ot->ot_rec_file, ot->ot_thread);
81
82 *eof = FALSE;
83 return OK;
84
85 failed:
86- if (ptr_locked)
87- XT_UNLOCK_MEMORY_PTR(ot->ot_rec_file, ot->ot_thread);
88 return FAILED;
89 }
90

Subscribers

People subscribed via source and target branches