perf(label_table): add TryGetIdByLabel to avoid exception overhead#1701
perf(label_table): add TryGetIdByLabel to avoid exception overhead#1701
Conversation
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: jinjiabao.jjb <jinjiabao.jjb@antgroup.com>
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request focuses on a performance enhancement by modifying the label lookup mechanism. It introduces a non-throwing alternative for retrieving inner IDs by label, which is then integrated into core algorithm components. This change is designed to reduce the computational cost associated with exception handling, particularly in scenarios where label lookups might frequently fail or involve removed labels, leading to a more efficient system overall. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
The pull request successfully refactors the GetIdByLabel functionality by introducing a new TryGetIdByLabel method. This change effectively replaces exception-based error handling with a return value for expected "not found" scenarios, which is a good practice for performance optimization in C++ by avoiding the overhead associated with exceptions. The new method is correctly integrated into the existing GetIdByLabel and other call sites, and comprehensive unit tests have been added to cover its behavior, including cases for successful lookups, non-existent labels, and removed labels.
|
@inabao this pull request cannot cherry-pick the branch 0.16 (CONFLICTS) please create a new pull request to the branch 0.16 . |
Cherry-pick of PR #1701 to 0.16 branch with adaptations for the different LabelTable structure in this branch.
Cherry-pick of PR #1701 to 0.16 branch with adaptations for the different LabelTable structure in this branch. Signed-off-by: jinjiabao.jjb <jinjiabao.jjb@antgroup.com>
Cherry-pick of PR #1701 to 0.16 branch with adaptations for the different LabelTable structure in this branch. Signed-off-by: jinjiabao.jjb <jinjiabao.jjb@antgroup.com>
|
@inabao this pull request cannot cherry-pick to the branch 0.18 (CONFLICTS), please create a nrew pull request to the branch 0.18 . |
Cherry-pick of PR #1701 to 0.16 branch with adaptations for the different LabelTable structure in this branch. Signed-off-by: jinjiabao.jjb <jinjiabao.jjb@antgroup.com>
…1701) Add TryGetIdByLabel method that returns std::pair<bool, InnerIdType> instead of throwing exceptions. This improves performance in hot paths by avoiding the overhead of exception handling. Changes: - Add TryGetIdByLabel() method to LabelTable class - Refactor GetIdByLabel() to use TryGetIdByLabel() internally - Update cal_distance_by_id() to use TryGetIdByLabel() instead of try-catch - Update SparseIndex::CalDistanceById() to use TryGetIdByLabel() Signed-off-by: jinjiabao.jjb <jinjiabao.jjb@antgroup.com>
…1701) (#1758) Add TryGetIdByLabel method that returns std::pair<bool, InnerIdType> instead of throwing exceptions. This improves performance in hot paths by avoiding the overhead of exception handling. Changes: - Add TryGetIdByLabel() method to LabelTable class - Refactor GetIdByLabel() to use TryGetIdByLabel() internally - Update cal_distance_by_id() to use TryGetIdByLabel() instead of try-catch - Update SparseIndex::CalDistanceById() to use TryGetIdByLabel() Signed-off-by: jinjiabao.jjb <jinjiabao.jjb@antgroup.com>
|
patch to 0.15 in #1769 |
…1701) Backport the non-throwing label lookup helper to 0.15 and update the CalDistanceById call paths to avoid exception-driven control flow. Keep the 0.15 branch shape intact by only applying the compatible caller changes on this branch. Signed-off-by: inabao <jinjiabao.jjb@antgroup.com> Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> (cherry picked from commit cafb34b)
…1701) (#1769) Backport the non-throwing label lookup helper to 0.15 and update the CalDistanceById call paths to avoid exception-driven control flow. (cherry picked from commit cafb34b) Signed-off-by: inabao <jinjiabao.jjb@antgroup.com> Co-authored-by: inabao <jinjiabao.jjb@antgroup.com> Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
close: #1729