Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
60 changes: 50 additions & 10 deletions storage/innobase/include/read0types.h
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,9 @@ class MVCC;
/** 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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ cppcoreguidelines-macro-usage ⚠️
macro MAX_TOP_ACTIVE_BYTES used to declare a constant; consider using a constexpr constant

#define MAX_SHORT_ACTIVE_BYTES 65536

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ cppcoreguidelines-macro-usage ⚠️
macro MAX_SHORT_ACTIVE_BYTES used to declare a constant; consider using a constexpr constant


class ReadView {
/** This is similar to a std::vector but it is not a drop
in replacement. It is specific to ReadView. */
Expand Down Expand Up @@ -173,14 +176,33 @@ class ReadView {

if (id >= m_low_limit_id) {
return (false);

} else if (m_ids.empty()) {
} else if (empty()) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ llvm-else-after-return ⚠️
do not use else after return

Suggested change
} else if (empty()) {
} if (empty()) {

return (true);
}

const ids_t::value_type *p = m_ids.data();
/* first search short bitmap */
if (m_has_short_actives && id >= m_short_min_id) {
if (id > m_short_max_id) {
return false;
}
unsigned int trim_id = id & 0x7FFFF;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ readability-magic-numbers ⚠️
0x7FFFF is a magic number; consider replacing it with a named constant

unsigned int trim_min_id = m_short_min_id & 0x7FFFF;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ readability-magic-numbers ⚠️
0x7FFFF is a magic number; consider replacing it with a named constant

unsigned int array_index = (trim_id >> 3);
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);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ readability-magic-numbers ⚠️
0x7 is a magic number; consider replacing it with a named constant

int is_value_set = top_active[array_index] & (1 << (7 - array_remainder));

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ readability-magic-numbers ⚠️
7 is a magic number; consider replacing it with a named constant

if (is_value_set) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ readability-implicit-bool-conversion ⚠️
implicit conversion int -> bool

return false;
} else {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ llvm-else-after-return ⚠️
do not use else after return

return true;
}
Comment on lines +196 to +200

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ readability-simplify-boolean-expr ⚠️
redundant boolean literal in conditional return statement

Suggested change
if (is_value_set) {
return false;
} else {
return true;
}
return is_value_set == 0;

}

const ids_t::value_type *p = m_long_ids.data();

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ readability-identifier-length ⚠️
variable name p is too short, expected at least 2 characters


return (!std::binary_search(p, p + m_ids.size(), id));
return (!std::binary_search(p, p + m_long_ids.size(), id));
}

/**
Expand Down Expand Up @@ -235,7 +257,18 @@ class ReadView {

/**
@return true if there are no transaction ids in the snapshot */
bool empty() const { return (m_ids.empty()); }
bool empty() const {
bool long_empty = m_long_ids.empty();
if (long_empty) {
if (!m_has_short_actives) {
return true;
} else {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ llvm-else-after-return ⚠️
do not use else after return

return false;
}
Comment on lines +263 to +267

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ readability-simplify-boolean-expr ⚠️
redundant boolean literal in conditional return statement

Suggested change
if (!m_has_short_actives) {
return true;
} else {
return false;
}
return !m_has_short_actives;

} else {
return false;
}
}

/**
Clones a read view object. The resulting read view has identical change
Expand Down Expand Up @@ -264,17 +297,19 @@ class ReadView {
fprintf(file, "Read view low limit trx n:o " TRX_ID_FMT "\n",
low_limit_no());
print_limits(file);
fprintf(file, "Read view individually stored trx ids:\n");
for (ulint i = 0; i < m_ids.size(); i++)
fprintf(file, "Read view trx id " TRX_ID_FMT "\n", m_ids.data()[i]);
fprintf(file, "Read view individually stored long trx ids:\n");
for (ulint i = 0; i < m_long_ids.size(); i++)
fprintf(file, "Read view trx id " TRX_ID_FMT "\n", m_long_ids.data()[i]);
}

bool is_cloned() const noexcept { return (m_cloned); }

private:
/**
Copy the transaction ids from the source vector */
inline void copy_trx_ids(const trx_ids_t &trx_ids);
inline void copy_long_trx_ids(const trx_ids_t &trx_ids,
trx_id_t min_short_id);
inline void copy_short_trx_ids();

/**
Opens a read view where exactly the transactions serialized before this
Expand Down Expand Up @@ -307,6 +342,7 @@ class ReadView {
ReadView &operator=(const ReadView &);

private:
unsigned char top_active[MAX_TOP_ACTIVE_BYTES];
/** The read should not see any transaction with trx id >= this
value. In other words, this is the "high water mark". */
trx_id_t m_low_limit_id;
Expand All @@ -322,7 +358,7 @@ class ReadView {

/** Set of RW transactions that was active when this snapshot
was taken */
ids_t m_ids;
ids_t m_long_ids;

/** The view does not need to see the undo logs for transactions
whose transaction number is strictly smaller (<) than this value:
Expand All @@ -337,6 +373,10 @@ class ReadView {
trx_id_t m_view_low_limit_no;
#endif /* UNIV_DEBUG */

trx_id_t m_short_min_id;
trx_id_t m_short_max_id;
bool m_has_short_actives;

/** AC-NL-RO transaction view that has been "closed". */
bool m_closed;

Expand Down
6 changes: 5 additions & 1 deletion storage/innobase/include/trx0sys.h
Original file line number Diff line number Diff line change
Expand Up @@ -553,7 +553,11 @@ struct trx_sys_t {
take a snapshot of these transactions whose changes are not visible to it.
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;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ misc-non-private-member-variables-in-classes ⚠️
member variable long_rw_trx_ids has public visibility

unsigned char short_rw_trx_ids_bitmap[MAX_SHORT_ACTIVE_BYTES];

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ misc-non-private-member-variables-in-classes ⚠️
member variable short_rw_trx_ids_bitmap has public visibility

int short_rw_trx_valid_number;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ misc-non-private-member-variables-in-classes ⚠️
member variable short_rw_trx_valid_number has public visibility

trx_id_t min_short_valid_id;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ misc-non-private-member-variables-in-classes ⚠️
member variable min_short_valid_id has public visibility

trx_id_t max_short_valid_id;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ misc-non-private-member-variables-in-classes ⚠️
member variable max_short_valid_id has public visibility


char pad7[ut::INNODB_CACHE_LINE_SIZE];

Expand Down
Loading