perf(label_table): add TryGetIdByLabel to avoid exception overhead by wxyucs · Pull Request #1769 · antgroup/vsag

wxyucs · 2026-03-30T06:57:03Z

Summary

backport the non-throwing LabelTable::TryGetIdByLabel helper onto 0.15
keep the 0.15 branch shape intact by applying only the compatible portion of cafb34b
add focused unit coverage for reverse-map and linear-scan lookup paths

gemini-code-assist

Code Review

This pull request introduces a TryGetIdByLabel method to the LabelTable class, providing a non-throwing way to retrieve an ID by its label, and refactors GetIdByLabel to utilize this new method. Additionally, unit tests were added to cover both the reverse map and linear lookup paths. Feedback suggests using std::optional instead of std::pair for the return type of TryGetIdByLabel to align with modern C++ idioms and improve safety.

gemini-code-assist · 2026-03-30T06:58:40Z

src/label_table.h

    GetIdByLabel(LabelType label) const {
+        auto [success, inner_id] = TryGetIdByLabel(label);
+        if (not success) {
+            throw std::runtime_error(fmt::format("label {} is not exists", label));
+        }
+        return inner_id;
+    }
+
+    inline std::pair<bool, InnerIdType>
+    TryGetIdByLabel(LabelType label) const noexcept {
        if (use_reverse_map_) {
-            if (this->label_remap_.count(label) == 0) {
-                throw std::runtime_error(fmt::format("label {} is not exists", label));
+            auto iter = this->label_remap_.find(label);
+            if (iter == this->label_remap_.end()) {
+                return {false, 0};
            }
-            return this->label_remap_.at(label);
+            return {true, iter->second};
        }
        auto result = std::find(label_table_.begin(), label_table_.end(), label);
        if (result == label_table_.end()) {
-            throw std::runtime_error(fmt::format("label {} is not exists", label));
+            return {false, 0};
        }
-        return result - label_table_.begin();
+        return {true, static_cast<InnerIdType>(result - label_table_.begin())};
    }


For representing an optional value, std::optional<InnerIdType> is more idiomatic and safer than std::pair<bool, InnerIdType> in modern C++ (C++17 and later). This avoids potential bugs where a consumer of this function might forget to check the boolean flag and incorrectly use the returned default value (0 in this case, which could be a valid ID).

This change also requires updating GetIdByLabel to handle the std::optional return type.

Note: You'll need to add #include <optional> at the top of the file.

GetIdByLabel(LabelType label) const { auto inner_id = TryGetIdByLabel(label); if (!inner_id) { throw std::runtime_error(fmt::format("label {} is not exists", label)); } return *inner_id; } inline std::optional<InnerIdType> TryGetIdByLabel(LabelType label) const noexcept { if (use_reverse_map_) { auto iter = this->label_remap_.find(label); if (iter == this->label_remap_.end()) { return std::nullopt; } return iter->second; } auto result = std::find(label_table_.begin(), label_table_.end(), label); if (result == label_table_.end()) { return std::nullopt; } return static_cast<InnerIdType>(result - label_table_.begin()); }

Copilot

Pull request overview

This PR backports a non-throwing label lookup API to the 0.15 branch to avoid exception overhead when probing for missing labels, and adds unit coverage for both lookup modes.

Changes:

Refactor LabelTable::GetIdByLabel to delegate to a new non-throwing helper.
Add LabelTable::TryGetIdByLabel returning {ok, inner_id} for both reverse-map and linear-scan paths.
Add unit tests covering success/failure for both reverse-map and non-reverse-map configurations.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File	Description
src/label_table.h	Adds `TryGetIdByLabel` and rewires `GetIdByLabel` to use it (preserving throwing behavior).
src/label_table_test.cpp	Adds focused Catch2 unit tests for `TryGetIdByLabel` under both lookup implementations.

Copilot · 2026-03-30T07:01:18Z

src/label_table.h

+    inline std::pair<bool, InnerIdType>
+    TryGetIdByLabel(LabelType label) const noexcept {
        if (use_reverse_map_) {


TryGetIdByLabel introduces a public std::pair return type, but this header doesn’t include the standard headers needed for the types it directly uses (std::pair/std::find/std::shared_ptr/std::function/std::tuple/std::runtime_error). This currently relies on transitive includes (e.g., from fmt/stream headers), which is fragile and can break with include-order changes. Add the appropriate <utility>, <algorithm>, <memory>, <functional>, <tuple>, and <stdexcept> includes in label_table.h.

Copilot · 2026-03-30T07:01:18Z

src/label_table.h

    GetIdByLabel(LabelType label) const {
+        auto [success, inner_id] = TryGetIdByLabel(label);
+        if (not success) {
+            throw std::runtime_error(fmt::format("label {} is not exists", label));


The thrown message "label {} is not exists" is grammatically incorrect / unclear for API consumers. Consider changing it to something like “label {} does not exist” (and keep the same phrasing anywhere else this error is surfaced).

Suggested change

throw std::runtime_error(fmt::format("label {} is not exists", label));

throw std::runtime_error(fmt::format("label {} does not exist", label));

Copilot · 2026-03-30T07:01:19Z

src/label_table.h

+            if (iter == this->label_remap_.end()) {
+                return {false, 0};
            }
-            return this->label_remap_.at(label);
+            return {true, iter->second};
        }
        auto result = std::find(label_table_.begin(), label_table_.end(), label);
        if (result == label_table_.end()) {
-            throw std::runtime_error(fmt::format("label {} is not exists", label));
+            return {false, 0};
        }


TryGetIdByLabel returns {false, 0} in the not-found case. Using a literal 0 here is a bit magic and couples behavior to the current InnerIdType representation; prefer returning a value-initialized InnerIdType{} (and consider whether the return value should be treated as unspecified when ok == false).

codecov · 2026-03-30T08:57:08Z

Codecov Report

❌ Patch coverage is 95.00000% with 1 line in your changes missing coverage. Please review.

@@            Coverage Diff             @@
##             0.15    #1769      +/-   ##
==========================================
+ Coverage   90.12%   90.21%   +0.09%     
==========================================
  Files         218      218              
  Lines       13537    13548      +11     
==========================================
+ Hits        12200    12223      +23     
+ Misses       1337     1325      -12

Flag	Coverage Δ
cpp	`90.21% <95.00%> (+0.09%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Components	Coverage Δ
common	`93.82% <100.00%> (+0.46%)`	⬆️
datacell	`91.36% <ø> (+0.26%)`	⬆️
index	`88.52% <88.88%> (-0.09%)`	⬇️
simd	`100.00% <ø> (ø)`

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3d876ca...d659222. Read the comment docs.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

…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)

inabao

LGTM

wxyucs requested review from LHT129, ShawnShawnYou, inabao and jiaweizone as code owners March 30, 2026 06:57

Copilot AI review requested due to automatic review settings March 30, 2026 06:57

pull-request-size bot added the size/M label Mar 30, 2026

Copilot started reviewing on behalf of wxyucs March 30, 2026 06:57 View session

gemini-code-assist bot reviewed Mar 30, 2026

View reviewed changes

Copilot AI reviewed Mar 30, 2026

View reviewed changes

wxyucs force-pushed the backport-cafb34b-to-0.15 branch from 73560df to 539bf21 Compare March 30, 2026 07:15

wxyucs added version/0.15 kind/improvement Code improvements (variable/function renaming, refactoring, etc. ) labels Mar 31, 2026

wxyucs force-pushed the backport-cafb34b-to-0.15 branch from 539bf21 to 6c891a7 Compare March 31, 2026 06:31

wxyucs mentioned this pull request Mar 31, 2026

perf(label_table): add TryGetIdByLabel to avoid exception overhead #1701

Merged

pull-request-size bot added size/L size/M and removed size/M size/L labels Mar 31, 2026

wxyucs force-pushed the backport-cafb34b-to-0.15 branch from 21ad084 to d659222 Compare March 31, 2026 11:07

inabao approved these changes Mar 31, 2026

View reviewed changes

wxyucs merged commit 4f9f38f into 0.15 Mar 31, 2026
24 checks passed

wxyucs deleted the backport-cafb34b-to-0.15 branch March 31, 2026 12:53

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

perf(label_table): add TryGetIdByLabel to avoid exception overhead#1769

perf(label_table): add TryGetIdByLabel to avoid exception overhead#1769
wxyucs merged 1 commit into0.15from
backport-cafb34b-to-0.15

wxyucs commented Mar 30, 2026

Uh oh!

gemini-code-assist bot left a comment

Uh oh!

gemini-code-assist bot Mar 30, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Mar 30, 2026

Uh oh!

Copilot AI Mar 30, 2026

Uh oh!

Copilot AI Mar 30, 2026

Uh oh!

codecov bot commented Mar 30, 2026 •

edited

Loading

Uh oh!

inabao left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

	throw std::runtime_error(fmt::format("label {} is not exists", label));
	throw std::runtime_error(fmt::format("label {} does not exist", label));

Conversation

wxyucs commented Mar 30, 2026

Summary

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist bot Mar 30, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Mar 30, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 30, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 30, 2026

Choose a reason for hiding this comment

Uh oh!

codecov bot commented Mar 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

inabao left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

codecov bot commented Mar 30, 2026 •

edited

Loading