Add a minimal micro-benchmark suite to validate filtering hot-path optimizations
メンテナーはふだん 1 日以内に返信
まだ誰も着手していません。
評価
- 難易度
- 4/5
- 見積もり時間
- 3〜5日
- 初心者へのやさしさ
- 35/100
- issue の種類
- 機能追加
- 明瞭さ
- 説明が足りない
- 活発さ
- 静か
- 技術スタック
- cpp
調査の方向性
まず既存の CMake と GoogleTest のセットアップを確認し、次に src/iceberg/**/ 配下の filtering パスとその metrics evaluators を追跡します。プロジェクトに合意済みの最小限の benchmark suite、デフォルトで無効になっている build option、そして filtering ステップを単独で測定する benchmark が用意されれば、作業は完了です。
索引モデルが issue の本文から書いたものです。
説明
While reading the scan-planning filtering path, I found a small optimization in the metrics evaluators. Using it as a concrete example to raise a broader question about how to validate this kind of change.
Proposed change
The metrics evaluators run per data file. Each predicate currently calls expr->reference() repeatedly, and reference() returns a shared_ptr via shared_from_this() — an atomic refcount bump every time. The StrictMetricsEvaluator macro even discards a dynamic_cast result only to re-fetch the same reference:
- #define RETURN_IF_NOT_REFERENCE(expr) \
- if (auto ref = dynamic_cast<BoundReference*>(expr.get()); ref == nullptr) { \
- return kRowsMightNotMatch; \
- }
+ #define BIND_REFERENCE_OR_RETURN(ref, expr) \
+ const auto* ref = dynamic_cast<const BoundReference*>((expr).get()); \
+ if (ref == nullptr) { \
+ return kRowsMightNotMatch; \
+ }
Result<bool> IsNull(const std::shared_ptr<Bound>& expr) override {
- RETURN_IF_NOT_REFERENCE(expr);
- int32_t id = expr->reference()->field().field_id();
+ BIND_REFERENCE_OR_RETURN(ref, expr);
+ int32_t id = ref->field().field_id();
...
Reusing the cast result drops the repeated virtual reference() calls (and their atomic ops) across every predicate, with no behavior change.
Expected benefit
The win is on the CPU-bound filtering step, evaluated in isolation. Scan planning as a whole is IO-bound, so on an e2e scan this kind of change is almost certainly unmeasurable — which is exactly why it needs to be measured on the filtering step alone.
Which raises the question: do we need a benchmark suite?
This is exactly the kind of change that's hard to justify without one. The repo has no benchmark infrastructure today, only the gtest suite. A minimal benchmark on the filtering path would let us measure such changes on the CPU-bound step alone, rather than guessing or claiming a win against IO-dominated planning.
So before going further:
- How should we add it — Google Benchmark fetched the same way googletest already is, behind an off-by-default CMake option?
- Where should it live — a top-level
benchmark/, or co-located undersrc/iceberg/**/?
I'm happy to put up a draft PR for a minimal suite + the filtering benchmark above once there's agreement on direction.
- 主要言語
- C++
- スター
- 226
- フォーク
- 132
- 平均マージ
- 1日 11時間
- マージ済み PR(30日)
- 27
環境構築
このプロジェクトには開発コンテナ、Dockerfile、コントリビューションガイドがありません。まず README を読み、一般的な手順ははじめてのコントリビューションガイドを参照してください。
はじめの一歩
- issue を最後まで読み、次にプロジェクトのコントリビューションガイドを読みます。
- 着手することを issue にコメントします — 二人が同じ作業をするのを防げます。
- リポジトリをフォークし、ブランチを切って変更します。
- issue 番号を参照したプルリクエストを送ります。
apache/iceberg-cpp のほかの issue
-
難易度 2/5 1〜3時間 初心者へのやさしさ 82/100
apache/iceberg-cpp#978 ·
メンテナーはふだん 1 日以内に返信
-
難易度 2/5 1〜3時間 初心者へのやさしさ 78/100
apache/iceberg-cpp#977 ·
メンテナーはふだん 1 日以内に返信
-
難易度 2/5 1〜3時間 初心者へのやさしさ 88/100
apache/iceberg-cpp#973 ·
メンテナーはふだん 1 日以内に返信
-
bug: expression JSON deserialization throws an uncaught exception on a non-string "type"/"term"オープン
難易度 3/5 1〜2日 初心者へのやさしさ 75/100
apache/iceberg-cpp#979 ·
メンテナーはふだん 1 日以内に返信
-
難易度 5/5 1週間以上 初心者へのやさしさ 20/100
apache/iceberg-cpp#959 · コメント 1 件 ·
メンテナーはふだん 1 日以内に返信
apache/iceberg-cpp の issue をすべて見る
似ている issue
-
Feature
難易度 1/5 1時間未満 初心者へのやさしさ 65/100
Narezzurri/OpenVPN-Config-Manager#95 ·
メンテナーはふだん 1 日以内に返信
-
bot-found bug priority: P1
難易度 2/5 1〜3時間 初心者へのやさしさ 68/100
madenvel/KalinkaPlayer#251 ·
-
難易度 2/5 1〜3時間 初心者へのやさしさ 78/100
sqlitebrowser/sqlitebrowser#4208 ·
-
ROSES ROSES - Student Review
難易度 2/5 1〜3時間 初心者へのやさしさ 82/100
メンテナーはふだん 1 日以内に返信
-
area/ysql kind/bug priority/medium
難易度 2/5 1〜3時間 初心者へのやさしさ 75/100
yugabyte/yugabyte-db#34552 ·
メンテナーはふだん 1 日以内に返信