Skip to content

refactor(dynamic_index): IDTranslator by simple vector for internal to external ID translation - #447

Draft
rfsaliev wants to merge 2 commits into
mainfrom
rfsaliev/refactor-translator
Draft

rfsaliev wants to merge 2 commits into
mainfrom
rfsaliev/refactor-translator

Conversation

@rfsaliev

Copy link
Copy Markdown
Member

This is the draft pull request to demonstrate ability to use simple vector for internal-to-external ID translation.
Benefits:

  • Potential performance improvements for id translation in search operations
  • Better compatibility with concurrent Vamana index translation (e.g. ability to get invalid_external_id).

…al-to-external mapping

- Added methods to estimate memory usage and manage internal capacity.
- Updated memory management in MutableVamanaIndex and DynamicFlatIndex to utilize new translator methods.
- Improved error handling for invalid external IDs and ensured proper resizing of internal structures.
@rfsaliev
rfsaliev requested a review from razdoburdin October 22, 2026 16:18

@razdoburdin razdoburdin left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Great idea!

It can be even more important for concurrent index. If grow stable storage is used for internal_to_external, it could allow to drop some locks, but it requires additional research.

// using bucket_type = tsl::detail_robin_hash::
// bucket_entry<std::pair<external_id_type, internal_id_type>, false>;
// external_to_internal_.bucket_count() * sizeof(bucket_type)
return size() * (sizeof(external_id_type) + sizeof(internal_id_type)) +

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

What's wrong with bucket_count() * sizeof(bucket_type) estimation? It should be ok for trivial types.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants