- 
                Notifications
    
You must be signed in to change notification settings  - Fork 502
 
PS-9647: MySQL Perf Improvements (readview) #5610
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: 8.0
Are you sure you want to change the base?
Conversation
https://perconadev.atlassian.net/browse/PS-9647 This patch introduces a new hybrid data structure for MVCC ReadView from Enhanced MySQL Typically, online transactions are short rather than long, and transaction IDs increase continuously. To leverage these characteristics, a hybrid data structure is used: a static array for consecutive short transaction IDs and a vector for long transactions. With a 2048-byte array, up to 16,384 consecutive active transaction IDs can be stored, each bit representing a transaction ID. The minimum short transaction ID is used to differentiate between short and long transactions. IDs smaller than this minimum go into the long transaction vector, while IDs equal to or greater than it are placed in the short transaction array. For an ID in changes_visible, if it is below the minimum short transaction ID, a direct query is made to the vector, which is efficient due to the generally small number of long transactions. If the ID is equal to or above the minimum short transaction ID, a bitwise query is performed, with a time complexity of O(1), compared to the previous O(log n) complexity. This improvement enhances efficiency and reduces cache migration between NUMA nodes, as O(1) queries typically complete within a single CPU time slice. - In the short_rw_trx_ids_bitmap structure, MAX_SHORT_ACTIVE_BYTES is set to 65536, theoretically accommodating up to 524,288 consecutive short transaction IDs. - If the limit is exceeded, the oldest short transaction IDs are converted into long transactions and stored in long_rw_trx_ids. - Global long and short transactions are distinguished by min_short_valid_id: IDs smaller than this value are treated as global long transactions, while IDs equal to or greater are considered global short transactions. During the copying process from the global active transaction list, the short_rw_trx_ids_bitmap structure, which uses only one bit per transaction ID, allows for much higher copying efficiency compared to the native MySQL solution. For example, with 1000 active transactions, the native MySQL version would require copying at least 8000 bytes, whereas the optimized solution may only need a few hundred bytes. This results in a significant improvement in copying efficiency.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Clang-Tidy found issue(s) with the introduced code (1/10)
| ut_ad(trx_sys_mutex_own()); | ||
| static inline void update_short_bitmamp(unsigned char *short_bitmap, | ||
| trx_id_t id, bool is_set_value) { | ||
| unsigned int trim_id = id & 0x7FFFF; | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
0x7FFFF is a magic number; consider replacing it with a named constant
| trx_id_t id, bool is_set_value) { | ||
| unsigned int trim_id = id & 0x7FFFF; | ||
| unsigned int array_index = (trim_id >> 3); | ||
| unsigned int array_remainder = trim_id & (0x7); | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
0x7 is a magic number; consider replacing it with a named constant
| unsigned int array_index = (trim_id >> 3); | ||
| unsigned int array_remainder = trim_id & (0x7); | ||
| if (is_set_value) { | ||
| short_bitmap[array_index] |= (1 << (7 - array_remainder)); | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
7 is a magic number; consider replacing it with a named constant
| if (is_set_value) { | ||
| short_bitmap[array_index] |= (1 << (7 - array_remainder)); | ||
| } else { | ||
| short_bitmap[array_index] &= (255 - (1 << (7 - array_remainder))); | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
7 is a magic number; consider replacing it with a named constant
| trx->id = | ||
| trx->preallocated_id ? trx->preallocated_id : trx_sys_allocate_trx_id(); | ||
| static inline trx_id_t find_smallest_short_active_trx_id( | ||
| unsigned char *short_bitmap, trx_id_t from, trx_id_t to) { | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pointer parameter short_bitmap can be pointer to const
| unsigned char *short_bitmap, trx_id_t from, trx_id_t to) { | |
| const unsigned char *short_bitmap, trx_id_t from, trx_id_t to) { | 
| trx_sys->rw_trx_ids.insert(upper_bound_it, trx->id); | ||
| trx_id_t start = from; | ||
| do { | ||
| unsigned int trim_id = start & 0x7FFFF; | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
0x7FFFF is a magic number; consider replacing it with a named constant
| do { | ||
| unsigned int trim_id = start & 0x7FFFF; | ||
| unsigned int array_index = (trim_id >> 3); | ||
| unsigned int array_remainder = trim_id & (0x7); | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
0x7 is a magic number; consider replacing it with a named constant
| unsigned int trim_id = start & 0x7FFFF; | ||
| unsigned int array_index = (trim_id >> 3); | ||
| unsigned int array_remainder = trim_id & (0x7); | ||
| int is_value_set = short_bitmap[array_index] & (1 << (7 - array_remainder)); | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
7 is a magic number; consider replacing it with a named constant
| unsigned int array_index = (trim_id >> 3); | ||
| unsigned int array_remainder = trim_id & (0x7); | ||
| int is_value_set = short_bitmap[array_index] & (1 << (7 - array_remainder)); | ||
| if (is_value_set) { | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
implicit conversion int -> bool
| if (is_value_set) { | |
| if (is_value_set != 0) { | 
| } else { | ||
| start++; | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do not use else after return
| } else { | |
| start++; | |
| } start++; | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Clang-Tidy found issue(s) with the introduced code (2/10)
| if (start > to) { | ||
| return to; | ||
| } | ||
| } | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do not use else after return
| } | |
| trx_sys->max_short_valid_id = trx->id; | ||
| } | ||
| 
               | 
          ||
| int64_t diff = trx_sys->max_short_valid_id - trx_sys->min_short_valid_id; | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
narrowing conversion from unsigned long to signed type int64_t (aka long) is implementation-defined
| trx_id_t old_id_start = trx_sys->min_short_valid_id; | ||
| trx_id_t max_short_valid_id = trx_sys->max_short_valid_id; | ||
| trx_id_t max_valid_id = max_short_valid_id; | ||
| max_valid_id = max_valid_id - ((max_valid_id & 0x7)); | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
0x7 is a magic number; consider replacing it with a named constant
| trx_id_t old_id_end = base - 1; | ||
| 
               | 
          ||
| for (trx_id_t id = old_id_start; id <= old_id_end; id++) { | ||
| unsigned int trim_id = id & 0x7FFFF; | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
0x7FFFF is a magic number; consider replacing it with a named constant
| for (trx_id_t id = old_id_start; id <= old_id_end; id++) { | ||
| unsigned int trim_id = id & 0x7FFFF; | ||
| unsigned int array_index = (trim_id >> 3); | ||
| unsigned int array_remainder = trim_id & (0x7); | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
0x7 is a magic number; consider replacing it with a named constant
| unsigned int array_index = (trim_id >> 3); | ||
| unsigned int array_remainder = trim_id & (0x7); | ||
| int is_value_set = | ||
| short_trx_id_bitmap[array_index] & (1 << (7 - array_remainder)); | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
7 is a magic number; consider replacing it with a named constant
| unsigned int array_remainder = trim_id & (0x7); | ||
| int is_value_set = | ||
| short_trx_id_bitmap[array_index] & (1 << (7 - array_remainder)); | ||
| if (is_value_set) { | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
implicit conversion int -> bool
| if (is_value_set) { | |
| if (is_value_set != 0) { | 
| trx_sys->long_rw_trx_ids.push_back(id); | ||
| trx_sys->short_rw_trx_valid_number--; | ||
| short_trx_id_bitmap[array_index] &= | ||
| (255 - (1 << (7 - array_remainder))); | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
7 is a magic number; consider replacing it with a named constant
| trx_id_t max_short_valid_id = trx_sys->max_short_valid_id; | ||
| 
               | 
          ||
| if (trx->id < min_short_valid_id) { | ||
| trx_ids_t::iterator it = | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
variable it is not initialized
| trx_ids_t::iterator it = | |
| trx_ids_t::iterator it = 0 = | 
| max_short_valid_id); | ||
| trx_sys->min_short_valid_id = candidate_min_short_valid_id; | ||
| } else { | ||
| int64_t diff = max_short_valid_id - min_short_valid_id; | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
narrowing conversion from unsigned long to signed type int64_t (aka long) is implementation-defined
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Clang-Tidy found issue(s) with the introduced code (3/10)
| if (diff >= MAX_SHORT_ACTIVE_BYTES) { | ||
| trx_id_t old_id_start = min_short_valid_id; | ||
| trx_id_t max_valid_id = max_short_valid_id; | ||
| max_valid_id = max_valid_id - ((max_valid_id & 0x7)); | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
0x7 is a magic number; consider replacing it with a named constant
| trx_id_t old_id_end = base - 1; | ||
| 
               | 
          ||
| for (trx_id_t id = old_id_start; id <= old_id_end; id++) { | ||
| unsigned int trim_id = id & 0x7FFFF; | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
0x7FFFF is a magic number; consider replacing it with a named constant
| for (trx_id_t id = old_id_start; id <= old_id_end; id++) { | ||
| unsigned int trim_id = id & 0x7FFFF; | ||
| unsigned int array_index = (trim_id >> 3); | ||
| unsigned int array_remainder = trim_id & (0x7); | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
0x7 is a magic number; consider replacing it with a named constant
| unsigned int array_index = (trim_id >> 3); | ||
| unsigned int array_remainder = trim_id & (0x7); | ||
| int is_value_set = | ||
| short_trx_id_bitmap[array_index] & (1 << (7 - array_remainder)); | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
7 is a magic number; consider replacing it with a named constant
| unsigned int array_remainder = trim_id & (0x7); | ||
| int is_value_set = | ||
| short_trx_id_bitmap[array_index] & (1 << (7 - array_remainder)); | ||
| if (is_value_set) { | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
implicit conversion int -> bool
| if (is_value_set) { | |
| if (is_value_set != 0) { | 
| trx_sys->long_rw_trx_ids.push_back(id); | ||
| trx_sys->short_rw_trx_valid_number--; | ||
| short_trx_id_bitmap[array_index] &= | ||
| (255 - (1 << (7 - array_remainder))); | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
7 is a magic number; consider replacing it with a named constant
| We should remove transactions from the list before committing in memory and | ||
| releasing locks to ensure right order of removal and consistent snapshot. */ | ||
| trx_ids_t rw_trx_ids; | ||
| trx_ids_t long_rw_trx_ids; | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
member variable long_rw_trx_ids has public visibility
| releasing locks to ensure right order of removal and consistent snapshot. */ | ||
| trx_ids_t rw_trx_ids; | ||
| trx_ids_t long_rw_trx_ids; | ||
| unsigned char short_rw_trx_ids_bitmap[MAX_SHORT_ACTIVE_BYTES]; | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
member variable short_rw_trx_ids_bitmap has public visibility
| trx_ids_t rw_trx_ids; | ||
| trx_ids_t long_rw_trx_ids; | ||
| unsigned char short_rw_trx_ids_bitmap[MAX_SHORT_ACTIVE_BYTES]; | ||
| int short_rw_trx_valid_number; | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
member variable short_rw_trx_valid_number has public visibility
| trx_ids_t long_rw_trx_ids; | ||
| unsigned char short_rw_trx_ids_bitmap[MAX_SHORT_ACTIVE_BYTES]; | ||
| int short_rw_trx_valid_number; | ||
| trx_id_t min_short_valid_id; | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
member variable min_short_valid_id has public visibility
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Clang-Tidy found issue(s) with the introduced code (4/10)
| unsigned char short_rw_trx_ids_bitmap[MAX_SHORT_ACTIVE_BYTES]; | ||
| int short_rw_trx_valid_number; | ||
| trx_id_t min_short_valid_id; | ||
| trx_id_t max_short_valid_id; | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
member variable max_short_valid_id has public visibility
| m_up_limit_id(), | ||
| m_creator_trx_id(), | ||
| m_ids(), | ||
| m_long_ids(), | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
initializer for member m_long_ids is redundant
| m_long_ids(), | |
| , | 
| m_long_ids.resize(size); | ||
| 
               | 
          ||
| ids_t::value_type *p = m_ids.data(); | ||
| ids_t::value_type *p = m_long_ids.data(); | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
variable name p is too short, expected at least 2 characters
| * `to` if no active transaction is found. | ||
| */ | ||
| static inline trx_id_t find_smallest_short_active_trx_id( | ||
| unsigned char *short_bitmap, trx_id_t from, trx_id_t to) { | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pointer parameter short_bitmap can be pointer to const
| unsigned char *short_bitmap, trx_id_t from, trx_id_t to) { | |
| const unsigned char *short_bitmap, trx_id_t from, trx_id_t to) { | 
| 
               | 
          ||
| trx_id_t start = from; | ||
| do { | ||
| unsigned int trim_id = start & 0x7FFFF; | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
0x7FFFF is a magic number; consider replacing it with a named constant
| do { | ||
| unsigned int trim_id = start & 0x7FFFF; | ||
| unsigned int array_index = (trim_id >> 3); | ||
| unsigned int array_remainder = trim_id & (0x7); | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
0x7 is a magic number; consider replacing it with a named constant
| unsigned int trim_id = start & 0x7FFFF; | ||
| unsigned int array_index = (trim_id >> 3); | ||
| unsigned int array_remainder = trim_id & (0x7); | ||
| int is_value_set = short_bitmap[array_index] & (1 << (7 - array_remainder)); | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
7 is a magic number; consider replacing it with a named constant
| unsigned int array_index = (trim_id >> 3); | ||
| unsigned int array_remainder = trim_id & (0x7); | ||
| int is_value_set = short_bitmap[array_index] & (1 << (7 - array_remainder)); | ||
| if (is_value_set) { | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
implicit conversion int -> bool
| if (is_value_set) { | |
| if (is_value_set != 0) { | 
| } else { | ||
| start++; | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do not use else after return
| } else { | |
| start++; | |
| } start++; | 
| if (start > to) { | ||
| return to; | ||
| } | ||
| } | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do not use else after return
| } | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Clang-Tidy found issue(s) with the introduced code (5/10)
| ut_ad(trx_sys_mutex_own()); | ||
| 
               | 
          ||
| unsigned char *short_trx_id_bitmap = trx_sys->short_rw_trx_ids_bitmap; | ||
| unsigned int start = trx_sys->min_short_valid_id & 0x7FFFF; | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
0x7FFFF is a magic number; consider replacing it with a named constant
| 
               | 
          ||
| unsigned char *short_trx_id_bitmap = trx_sys->short_rw_trx_ids_bitmap; | ||
| unsigned int start = trx_sys->min_short_valid_id & 0x7FFFF; | ||
| unsigned int end = trx_sys->max_short_valid_id & 0x7FFFF; | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
0x7FFFF is a magic number; consider replacing it with a named constant
| unsigned int array_index_end = (end >> 3); | ||
| 
               | 
          ||
| if (array_index_start <= array_index_end) { | ||
| int diff = array_index_end - array_index_start + 1; | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
narrowing conversion from unsigned int to signed type int is implementation-defined
| trx_id_t old_id_start = trx_sys->min_short_valid_id; | ||
| trx_id_t max_short_valid_id = trx_sys->max_short_valid_id; | ||
| trx_id_t max_valid_id = max_short_valid_id; | ||
| max_valid_id = max_valid_id - ((max_valid_id & 0x7)); | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
0x7 is a magic number; consider replacing it with a named constant
| trx_id_t old_id_end = base - 1; | ||
| 
               | 
          ||
| for (trx_id_t id = old_id_start; id <= old_id_end; id++) { | ||
| unsigned int trim_id = id & 0x7FFFF; | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
0x7FFFF is a magic number; consider replacing it with a named constant
| for (trx_id_t id = old_id_start; id <= old_id_end; id++) { | ||
| unsigned int trim_id = id & 0x7FFFF; | ||
| unsigned int array_index = (trim_id >> 3); | ||
| unsigned int array_remainder = trim_id & (0x7); | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
0x7 is a magic number; consider replacing it with a named constant
| unsigned int array_index = (trim_id >> 3); | ||
| unsigned int array_remainder = trim_id & (0x7); | ||
| int is_value_set = | ||
| short_trx_id_bitmap[array_index] & (1 << (7 - array_remainder)); | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
7 is a magic number; consider replacing it with a named constant
| unsigned int array_remainder = trim_id & (0x7); | ||
| int is_value_set = | ||
| short_trx_id_bitmap[array_index] & (1 << (7 - array_remainder)); | ||
| if (is_value_set) { | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
implicit conversion int -> bool
| if (is_value_set) { | |
| if (is_value_set != 0) { | 
| trx_sys->long_rw_trx_ids.push_back(id); | ||
| trx_sys->short_rw_trx_valid_number--; | ||
| short_trx_id_bitmap[array_index] &= | ||
| (255 - (1 << (7 - array_remainder))); | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
7 is a magic number; consider replacing it with a named constant
| } | ||
| } | ||
| 
               | 
          ||
| start = candidate_min_short_valid_id & 0x7FFFF; | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
0x7FFFF is a magic number; consider replacing it with a named constant
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Clang-Tidy found issue(s) with the introduced code (6/10)
| } | ||
| 
               | 
          ||
| start = candidate_min_short_valid_id & 0x7FFFF; | ||
| end = max_short_valid_id & 0x7FFFF; | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
0x7FFFF is a magic number; consider replacing it with a named constant
| 
               | 
          ||
| array_index_start = (start >> 3); | ||
| array_index_end = (end >> 3); | ||
| diff = array_index_end - array_index_start + 1; | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
narrowing conversion from unsigned int to signed type int is implementation-defined
| } | ||
| } else { | ||
| int diff = MAX_SHORT_ACTIVE_BYTES - array_index_start; | ||
| int total = diff + array_index_end + 1; | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
narrowing conversion from unsigned int to signed type int is implementation-defined
| if (array_index_end > MAX_TOP_ACTIVE_BYTES) { | ||
| trx_id_t max_short_valid_id = trx_sys->max_short_valid_id; | ||
| trx_id_t max_valid_id = max_short_valid_id; | ||
| max_valid_id = max_valid_id - ((max_valid_id & 0x7)); | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
0x7 is a magic number; consider replacing it with a named constant
| 
               | 
          ||
| trx_id_t old_id_end = base - 1; | ||
| for (trx_id_t id = old_id_start; id <= old_id_end; id++) { | ||
| unsigned int trim_id = id & 0x7FFFF; | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
0x7FFFF is a magic number; consider replacing it with a named constant
| for (trx_id_t id = old_id_start; id <= old_id_end; id++) { | ||
| unsigned int trim_id = id & 0x7FFFF; | ||
| unsigned int array_index = (trim_id >> 3); | ||
| unsigned int array_remainder = trim_id & (0x7); | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
0x7 is a magic number; consider replacing it with a named constant
| unsigned int array_index = (trim_id >> 3); | ||
| unsigned int array_remainder = trim_id & (0x7); | ||
| int is_value_set = | ||
| short_trx_id_bitmap[array_index] & (1 << (7 - array_remainder)); | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
7 is a magic number; consider replacing it with a named constant
| unsigned int array_remainder = trim_id & (0x7); | ||
| int is_value_set = | ||
| short_trx_id_bitmap[array_index] & (1 << (7 - array_remainder)); | ||
| if (is_value_set) { | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
implicit conversion int -> bool
| if (is_value_set) { | |
| if (is_value_set != 0) { | 
| trx_sys->long_rw_trx_ids.push_back(id); | ||
| trx_sys->short_rw_trx_valid_number--; | ||
| short_trx_id_bitmap[array_index] &= | ||
| (255 - (1 << (7 - array_remainder))); | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
7 is a magic number; consider replacing it with a named constant
| } | ||
| } | ||
| 
               | 
          ||
| start = candidate_min_short_valid_id & 0x7FFFF; | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
0x7FFFF is a magic number; consider replacing it with a named constant
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Clang-Tidy found issue(s) with the introduced code (7/10)
| } | ||
| 
               | 
          ||
| start = candidate_min_short_valid_id & 0x7FFFF; | ||
| end = max_short_valid_id & 0x7FFFF; | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
0x7FFFF is a magic number; consider replacing it with a named constant
| 
               | 
          ||
| array_index_start = (start >> 3); | ||
| array_index_end = (end >> 3); | ||
| diff = array_index_end - array_index_start + 1; | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
narrowing conversion from unsigned int to signed type int is implementation-defined
| } else { | ||
| trx_id_t max_short_valid_id = trx_sys->max_short_valid_id; | ||
| trx_id_t max_valid_id = max_short_valid_id; | ||
| max_valid_id = max_valid_id - ((max_valid_id & 0x7)); | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
0x7 is a magic number; consider replacing it with a named constant
| trx_id_t old_id_end = base - 1; | ||
| 
               | 
          ||
| for (trx_id_t id = old_id_start; id <= old_id_end; id++) { | ||
| unsigned int trim_id = id & 0x7FFFF; | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
0x7FFFF is a magic number; consider replacing it with a named constant
| for (trx_id_t id = old_id_start; id <= old_id_end; id++) { | ||
| unsigned int trim_id = id & 0x7FFFF; | ||
| unsigned int array_index = (trim_id >> 3); | ||
| unsigned int array_remainder = trim_id & (0x7); | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
0x7 is a magic number; consider replacing it with a named constant
| unsigned int array_index = (trim_id >> 3); | ||
| unsigned int array_remainder = trim_id & (0x7); | ||
| int is_value_set = | ||
| short_trx_id_bitmap[array_index] & (1 << (7 - array_remainder)); | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
7 is a magic number; consider replacing it with a named constant
| unsigned int array_remainder = trim_id & (0x7); | ||
| int is_value_set = | ||
| short_trx_id_bitmap[array_index] & (1 << (7 - array_remainder)); | ||
| if (is_value_set) { | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
implicit conversion int -> bool
| if (is_value_set) { | |
| if (is_value_set != 0) { | 
| trx_sys->long_rw_trx_ids.push_back(id); | ||
| trx_sys->short_rw_trx_valid_number--; | ||
| short_trx_id_bitmap[array_index] &= | ||
| (255 - (1 << (7 - array_remainder))); | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
7 is a magic number; consider replacing it with a named constant
| } | ||
| } | ||
| 
               | 
          ||
| start = candidate_min_short_valid_id & 0x7FFFF; | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
0x7FFFF is a magic number; consider replacing it with a named constant
| } | ||
| 
               | 
          ||
| start = candidate_min_short_valid_id & 0x7FFFF; | ||
| end = max_short_valid_id & 0x7FFFF; | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
0x7FFFF is a magic number; consider replacing it with a named constant
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Clang-Tidy found issue(s) with the introduced code (8/10)
| array_index_end = (end >> 3); | ||
| 
               | 
          ||
| if (array_index_start <= array_index_end) { | ||
| diff = array_index_end - array_index_start + 1; | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
narrowing conversion from unsigned int to signed type int is implementation-defined
| ::memmove(top_active, &short_trx_id_bitmap[array_index_start], diff); | ||
| } else { | ||
| diff = MAX_SHORT_ACTIVE_BYTES - array_index_start; | ||
| unsigned char *p = top_active; | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
variable name p is too short, expected at least 2 characters
| } | ||
| } | ||
| } else { | ||
| unsigned char *p = top_active; | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
variable name p is too short, expected at least 2 characters
| 
               | 
          ||
| if (!trx_sys->rw_trx_ids.empty()) { | ||
| copy_trx_ids(trx_sys->rw_trx_ids); | ||
| if (trx_sys->short_rw_trx_valid_number) { | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
implicit conversion int -> bool
| if (trx_sys->short_rw_trx_valid_number) { | |
| if (trx_sys->short_rw_trx_valid_number != 0) { | 
| if (!m_long_ids.empty()) { | ||
| m_up_limit_id = m_long_ids.front(); | ||
| } else { | ||
| if (trx_sys->short_rw_trx_valid_number) { | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
implicit conversion int -> bool
| if (trx_sys->short_rw_trx_valid_number) { | |
| if (trx_sys->short_rw_trx_valid_number != 0) { | 
| if (!other.m_ids.empty()) { | ||
| const ids_t::value_type *p = other.m_ids.data(); | ||
| if (other.m_has_short_actives) { | ||
| unsigned int max_trim_id = other.m_short_max_id & 0x7FFFF; | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
0x7FFFF is a magic number; consider replacing it with a named constant
| const ids_t::value_type *p = other.m_ids.data(); | ||
| if (other.m_has_short_actives) { | ||
| unsigned int max_trim_id = other.m_short_max_id & 0x7FFFF; | ||
| unsigned int min_trim_id = other.m_short_min_id & 0x7FFFF; | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
0x7FFFF is a magic number; consider replacing it with a named constant
| unsigned int max_array_index = (max_trim_id >> 3); | ||
| unsigned int min_array_index = (min_trim_id >> 3); | ||
| int diff = | ||
| (MAX_SHORT_ACTIVE_BYTES + max_array_index - min_array_index + 1) % | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
narrowing conversion from unsigned int to signed type int is implementation-defined
| } | ||
| 
               | 
          ||
| if (!other.m_long_ids.empty()) { | ||
| const ids_t::value_type *p = other.m_long_ids.data(); | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
variable name p is too short, expected at least 2 characters
| if (m_creator_trx_id > 0) { | ||
| m_ids.insert(m_creator_trx_id); | ||
| if (m_short_min_id <= m_creator_trx_id) { | ||
| unsigned int trim_id = m_creator_trx_id & 0x7FFFF; | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
0x7FFFF is a magic number; consider replacing it with a named constant
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Clang-Tidy found issue(s) with the introduced code (9/10)
| m_ids.insert(m_creator_trx_id); | ||
| if (m_short_min_id <= m_creator_trx_id) { | ||
| unsigned int trim_id = m_creator_trx_id & 0x7FFFF; | ||
| unsigned int trim_min_id = m_short_min_id & 0x7FFFF; | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
0x7FFFF is a magic number; consider replacing it with a named constant
| unsigned int array_min_index = (trim_min_id >> 3); | ||
| array_index = (MAX_SHORT_ACTIVE_BYTES + array_index - array_min_index) % | ||
| MAX_TOP_ACTIVE_BYTES; | ||
| unsigned int array_remainder = trim_id & (0x7); | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
0x7 is a magic number; consider replacing it with a named constant
| array_index = (MAX_SHORT_ACTIVE_BYTES + array_index - array_min_index) % | ||
| MAX_TOP_ACTIVE_BYTES; | ||
| unsigned int array_remainder = trim_id & (0x7); | ||
| top_active[array_index] |= (1 << (7 - array_remainder)); | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
7 is a magic number; consider replacing it with a named constant
| unsigned int array_remainder = trim_id & (0x7); | ||
| top_active[array_index] |= (1 << (7 - array_remainder)); | ||
| } else { | ||
| m_long_ids.insert(m_creator_trx_id); | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
narrowing conversion from trx_id_t (aka unsigned long) to signed type ReadView::ids_t::value_type (aka int) is implementation-defined
| /** Read view lists the trx ids of those transactions for which a consistent | ||
| read should not see the modifications to the database. */ | ||
| 
               | 
          ||
| #define MAX_TOP_ACTIVE_BYTES 8192 | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
macro MAX_TOP_ACTIVE_BYTES used to declare a constant; consider using a constexpr constant
| read should not see the modifications to the database. */ | ||
| 
               | 
          ||
| #define MAX_TOP_ACTIVE_BYTES 8192 | ||
| #define MAX_SHORT_ACTIVE_BYTES 65536 | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
macro MAX_SHORT_ACTIVE_BYTES used to declare a constant; consider using a constexpr constant
| return (false); | ||
| 
               | 
          ||
| } else if (m_ids.empty()) { | ||
| } else if (empty()) { | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do not use else after return
| } else if (empty()) { | |
| } if (empty()) { | 
| if (id > m_short_max_id) { | ||
| return false; | ||
| } | ||
| unsigned int trim_id = id & 0x7FFFF; | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
0x7FFFF is a magic number; consider replacing it with a named constant
| return false; | ||
| } | ||
| unsigned int trim_id = id & 0x7FFFF; | ||
| unsigned int trim_min_id = m_short_min_id & 0x7FFFF; | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
0x7FFFF is a magic number; consider replacing it with a named constant
| unsigned int array_min_index = (trim_min_id >> 3); | ||
| array_index = (MAX_SHORT_ACTIVE_BYTES + array_index - array_min_index) % | ||
| MAX_TOP_ACTIVE_BYTES; | ||
| unsigned int array_remainder = trim_id & (0x7); | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
0x7 is a magic number; consider replacing it with a named constant
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Clang-Tidy found issue(s) with the introduced code (10/10)
| array_index = (MAX_SHORT_ACTIVE_BYTES + array_index - array_min_index) % | ||
| MAX_TOP_ACTIVE_BYTES; | ||
| unsigned int array_remainder = trim_id & (0x7); | ||
| int is_value_set = top_active[array_index] & (1 << (7 - array_remainder)); | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
7 is a magic number; consider replacing it with a named constant
| MAX_TOP_ACTIVE_BYTES; | ||
| unsigned int array_remainder = trim_id & (0x7); | ||
| int is_value_set = top_active[array_index] & (1 << (7 - array_remainder)); | ||
| if (is_value_set) { | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
implicit conversion int -> bool
| if (is_value_set) { | ||
| return false; | ||
| } else { | ||
| return true; | ||
| } | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
redundant boolean literal in conditional return statement
| if (is_value_set) { | |
| return false; | |
| } else { | |
| return true; | |
| } | |
| return is_value_set == 0; | 
| int is_value_set = top_active[array_index] & (1 << (7 - array_remainder)); | ||
| if (is_value_set) { | ||
| return false; | ||
| } else { | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do not use else after return
| } | ||
| } | ||
| 
               | 
          ||
| const ids_t::value_type *p = m_long_ids.data(); | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
variable name p is too short, expected at least 2 characters
| if (!m_has_short_actives) { | ||
| return true; | ||
| } else { | ||
| return false; | ||
| } | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
redundant boolean literal in conditional return statement
| if (!m_has_short_actives) { | |
| return true; | |
| } else { | |
| return false; | |
| } | |
| return !m_has_short_actives; | 
| if (long_empty) { | ||
| if (!m_has_short_actives) { | ||
| return true; | ||
| } else { | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do not use else after return
https://perconadev.atlassian.net/browse/PS-9647
This patch introduces a new hybrid data structure for MVCC ReadView from Enhanced MySQL
Typically, online transactions are short rather than long, and transaction IDs increase continuously. To leverage these characteristics, a hybrid data structure is used: a static array for consecutive short transaction IDs and a vector for long transactions. With a 2048-byte array, up to 16,384 consecutive active transaction IDs can be stored, each bit representing a transaction ID.
The minimum short transaction ID is used to differentiate between short and long transactions.
IDs smaller than this minimum go into the long transaction vector, while IDs equal to or greater than it are placed in the short transaction array.
For an ID in changes_visible, if it is below the minimum short transaction ID, a direct query is made to the vector, which is efficient due to the generally small number of long transactions.
If the ID is equal to or above the minimum short transaction ID, a bitwise query is performed, with a time complexity of O(1), compared to the previous O(log n) complexity. This improvement enhances efficiency and reduces cache migration between NUMA nodes, as O(1) queries typically complete within a single CPU time slice.
In the short_rw_trx_ids_bitmap structure, MAX_SHORT_ACTIVE_BYTES is set to 65536, theoretically accommodating up to 524,288 consecutive short transaction IDs.
If the limit is exceeded, the oldest short transaction IDs are converted into long transactions and stored in long_rw_trx_ids.
Global long and short transactions are distinguished by min_short_valid_id: IDs smaller than this value are treated as global long transactions, while IDs equal to or greater are considered global short transactions.
During the copying process from the global active transaction list, the short_rw_trx_ids_bitmap structure, which uses only one bit per transaction ID, allows for much higher copying efficiency compared to the native MySQL solution.
For example, with 1000 active transactions, the native MySQL version would require copying at least 8000 bytes, whereas the optimized solution may only need a few hundred bytes. This results in a significant improvement in copying efficiency.