Skip to content

MDEV-35190 HASH_SEARCH duplicates effort before HASH_INSERT or HASH_DELETE - #3627

Merged
dr-m merged 3 commits into
10.6from
10.6-MDEV-35190
Nov 21, 2024
Merged

MDEV-35190 HASH_SEARCH duplicates effort before HASH_INSERT or HASH_DELETE#3627
dr-m merged 3 commits into
10.6from
10.6-MDEV-35190

Conversation

@dr-m

@dr-m dr-m commented Nov 12, 2024

Copy link
Copy Markdown
Contributor

This supercedes #3593, which was accidentally closed and can’t be resurrected.

  • The Jira issue number for this PR is: MDEV-35190 and MDEV-35247

Description

The HASH_ macros are unnecessarily obfuscating the logic, so we had better replace them.

hash_cell_t::remove(): Implement most of the HASH_DELETE logic. This is based on buf_pool_t::page_hash_table::remove().

xb_filter_hash_free(): Avoid any hash table lookup; just traverse the hash bucket chains and free each element.

rm_if_not_found(): Make use of find_filter_in_hashtable().

dict_sys_t::acquire_temporary_table(), dict_sys_t::find_table(): Define non-inline to avoid unnecessary code duplication.

dict_sys_t::add(dict_table_t *table), dict_table_rename_in_cache(): Look for duplicate while finding the insert position.

dict_table_change_id_in_cache(): Merged to the only caller row_discard_tablespace().

hash_insert(): Helper function of dict_sys_t::resize().

fil_space_t::create(): Look for a duplicate (and crash if found) when searching for the insert position.

lock_rec_discard(): Take the hash array cell as a parameter to avoid a duplicated lookup.

lock_rec_free_all_from_discard_page(): Remove a parameter.

ut_fold_ull(): For SIZEOF_SIZE_T < 8, we simulate universal hashing (Carter and Wegman, 1977) by pretending that SIZE_T_MAX + 1 is a prime. In other words, we implement a Rabin–Karp rolling hash algorithm similar to java.lang.String.hashCode(). This is used for representing 64-bit dict_index_t::id or dict_table_t::id in the native word size.

If SIZEOF_SIZE_T >= 8, we just use an identity mapping instead of ut_fold_ull().

ut_hash_ulint(): Remove. The exclusive OR before a modulus operation does not serve any useful purpose; it is only obfuscating and wasting some code. mysql/mysql-server@b11a175 included a similar change.

Release Notes

Some InnoDB hash table operations were simplified to slightly improve performance.

How can this PR be tested?

This low level code should be well covered by the regression test suite. Some additional performance and stress testing would be helpful.

Basing the PR against the correct MariaDB version

  • This is a new feature or a refactoring, and the PR is based against the main branch.
  • This is a bug fix, and the PR is based against the earliest maintained branch in which the bug can be reproduced.

This depends on #3584 which was merged to 10.6.

PR quality check

  • I checked the CODING_STANDARDS.md file and my PR conforms to this where appropriate.
  • For any trivial modifications to the PR, I am ok with the reviewer making the changes themselves.

@dr-m dr-m self-assigned this Nov 12, 2024
@CLAassistant

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

Comment thread storage/innobase/dict/dict0dict.cc Outdated
Comment thread extra/mariabackup/xtrabackup.cc Outdated
@dr-m
dr-m force-pushed the 10.6-MDEV-35190 branch 3 times, most recently from 13d590e to d4b37ae Compare November 14, 2024 07:01

@mariadb-DebarunBanerjee mariadb-DebarunBanerjee left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks @dr-m for keeping the hash table abstraction and still keeping the refactoring scope limited. Looks good to me.

dr-m added 3 commits November 21, 2024 08:59
…ELETE

The HASH_ macros are unnecessarily obfuscating the logic,
so we had better replace them.

hash_cell_t::search(): Implement most of the HASH_DELETE logic,
for a subsequent insert or remove().

hash_cell_t::remove(): Remove an element.

hash_cell_t::find(): Implement the HASH_SEARCH logic.

xb_filter_hash_free(): Avoid any hash table lookup;
just traverse the hash bucket chains and free each element.

xb_register_filter_entry(): Search databases_hash only once.

rm_if_not_found(): Make use of find_filter_in_hashtable().

dict_sys_t::acquire_temporary_table(), dict_sys_t::find_table():
Define non-inline to avoid unnecessary code duplication.

dict_sys_t::add(dict_table_t *table), dict_table_rename_in_cache():
Look for duplicate while finding the insert position.

dict_table_change_id_in_cache(): Merged to the only caller
row_discard_tablespace().

hash_insert(): Helper function of dict_sys_t::resize().

fil_space_t::create(): Look for a duplicate (and crash if found)
when searching for the insert position.

lock_rec_discard(): Take the hash array cell as a parameter
to avoid a duplicated lookup.

lock_rec_free_all_from_discard_page(): Remove a parameter.

Reviewed by: Debarun Banerjee
ut_fold_ull(): For SIZEOF_SIZE_T < 8, we simulate universal hashing
(Carter and Wegman, 1977) by pretending that SIZE_T_MAX + 1
is a prime. In other words, we implement a Rabin–Karp rolling
hash algorithm similar to java.lang.String.hashCode().
This is used for representing 64-bit dict_index_t::id or
dict_table_t::id in the native word size.

For SIZEOF_SIZE_T >= 8, we just use an identity mapping.

Reviewed by: Debarun Banerjee
ut_hash_ulint(): Remove. The exclusive OR before a modulus operation
does not serve any useful purpose; it is only obfuscating code and
wasting some CPU cycles.

Reviewed by: Debarun Banerjee
@dr-m
dr-m merged commit df3855a into 10.6 Nov 21, 2024
@dr-m
dr-m deleted the 10.6-MDEV-35190 branch November 21, 2024 10:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants