Uploaded image for project: 'MariaDB Server'
  1. MariaDB Server
  2. MDEV-35701

trx_t::autoinc_locks causes unnecessary dynamic memory allocation

Details

    Description

      The data structure trx_t::autoinc_locks is of type ib_vector_t*. It is unconditionally allocated in trx_create() even when no AUTO_INCREMENT locks will be used by the transaction, even for read-only transactions.

      We had better make use of a template that was introduced in MDEV-30289:

      small_vector<lock_t*, 4> autoinc_locks;

      The data elements are trivially assignable and allocated separately; the first 8 table locks of a transaction would use the trx->lock.table_pool in lock_table_create(). Any data-modifying transaction will typically also require a table IX lock in addition to the auto-increment lock. That is why 4 would be a reasonable fixed size.

      Only if a transaction involved more than 4 auto-increment locks, we would dynamically allocate a larger vector, similar to how the 16-element mtr_t::m_memo works.

      Attachments

        Issue Links

          is blocked by

          Bug - A problem which impairs or prevents the functions of the product. MDEV-30289 mtr_t::m_memo is causing frequent memory heap operations

          • Blocker - Blocks development and/or testing work, production could not run.
          • Closed
          relates to

          Bug - A problem which impairs or prevents the functions of the product. MDEV-35702 ReadViewBase::m_ids had better use an initial fixed part

          • Major - Major loss of function.
          • Open

          Activity

            Ascending order - Click to sort in descending order
            marko Marko Mäkelä added a comment -

            I was too optimistic; there are some test failures that I will need to address.

            marko Marko Mäkelä added a comment - I was too optimistic; there are some test failures that I will need to address.
            debarun Debarun Banerjee added a comment -

            Mainly we are replacing ` ib_vector_t* autoinc_locks; ` with `small_vector`. So, the possible impact mainly would depend on the stability of `small_vector` implementation. I think we need to test it well. I added one comment on passing R value while adding the lock entry which doesn't seem completely right. I need a little more time to complete the review.

            debarun Debarun Banerjee added a comment - Mainly we are replacing ` ib_vector_t* autoinc_locks; ` with `small_vector`. So, the possible impact mainly would depend on the stability of `small_vector` implementation. I think we need to test it well. I added one comment on passing R value while adding the lock entry which doesn't seem completely right. I need a little more time to complete the review.
            debarun Debarun Banerjee added a comment -

            markoI am done with the review and should be able to approve once you check/reply/address the comments. I think the chances of regression is from the small_vector implementation.

            debarun Debarun Banerjee added a comment - marko I am done with the review and should be able to approve once you check/reply/address the comments. I think the chances of regression is from the small_vector implementation.
            marko Marko Mäkelä added a comment -

            Thank you for the first review. I spent quite some time in an attempt to improve code coverage. I am able to occasionally (nondeterministically) cover the out-of-order lock release in lock_table_remove_autoinc_lock(), but not any AUTO_INCREMENT lock release in lock_cancel_waiting_and_release().

            marko Marko Mäkelä added a comment - Thank you for the first review. I spent quite some time in an attempt to improve code coverage. I am able to occasionally (nondeterministically) cover the out-of-order lock release in lock_table_remove_autoinc_lock() , but not any AUTO_INCREMENT lock release in lock_cancel_waiting_and_release() .
            debarun Debarun Banerjee added a comment -

            Thanks for the changes. The latest code looks good to me.

            debarun Debarun Banerjee added a comment - Thanks for the changes. The latest code looks good to me.

            People

              marko Marko Mäkelä
              marko Marko Mäkelä
              Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

                Created:
                Updated:
                Resolved:

                Git Integration

                  Error rendering 'com.xiplink.jira.git.jira_git_plugin:git-issue-webpanel'. Please contact your Jira administrators.