Skip to content
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

MyRocks: pass GL_INDEX_ID around only by value #1504

Open
wants to merge 2 commits into
base: fb-mysql-8.0.32
Choose a base branch
from

Conversation

laurynas-biveinis
Copy link
Contributor

The size of this structure is 64 bits and passing it around by references and
pointers only adds one level of indirection for no gain.

  • Convert all its reference uses to direct value ones
  • Make rdb_netbuf_read_gl_index return the value directly instead of writing to
    a pointer output parameter.
  • Remove a couple of dead declarations
  • Apply const, auto, [[nodiscard]] to touched code. Handle
    local_dict_manager->put_auto_incr_val failure in
    ha_rocksdb::commit_inplace_alter_table by crashing, like other error handling
    code in this method does.
  • Add a couple of reserve calls before inserting into C++ containers when the
    final size is known in advance.
  • Reorder Rdb_index_info fields to remove padding gaps

The size of this structure is 64 bits and passing it around by references and
pointers only adds one level of indirection for no gain.

- Convert all its reference uses to direct value ones
- Make rdb_netbuf_read_gl_index return the value directly instead of writing to
  a pointer output parameter.
- Remove a couple of dead declarations
- Apply const, auto, [[nodiscard]] to touched code. Handle
  local_dict_manager->put_auto_incr_val failure in
  ha_rocksdb::commit_inplace_alter_table by crashing, like other error handling
  code in this method does.
- Add a couple of reserve calls before inserting into C++ containers when the
  final size is known in advance.
- Reorder Rdb_index_info fields to remove padding gaps
@facebook-github-bot
Copy link

@luqun has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants