Add a minimal micro-benchmark suite to validate filtering hot-path optimizations
Les mainteneurs répondent en général sous 2 jours
Personne n'a encore pris cette issue.
- #823 par @SURYAS1306 — fermée sans fusion
Évaluation
- Difficulté
- 4/5
- Temps estimé
- 3-5 jours
- Accessibilité débutants
- 35/100
- Type d'issue
- Fonctionnalité
- Clarté
- À clarifier
- Activité
- Calme
- Stack technique
- cpp
- Domaine
- build-system, performance, tooling
Piste de recherche
Commencez par examiner la configuration existante de CMake et GoogleTest, puis suivez le chemin de filtrage sous src/iceberg/**/ et ses évaluateurs de métriques. Le travail est terminé lorsque le projet dispose d’une suite minimale de benchmarks approuvée, d’une option de compilation désactivée par défaut et d’un benchmark qui mesure l’étape de filtrage de manière isolée.
Rédigé par le modèle d'indexation à partir du texte de l'issue.
Description
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.
- Langage dominant
- C++
- Étoiles
- 226
- Forks
- 133
- Merge moyen
- 5 j 10 h
- PR mergées (30 j)
- 33
Préparer son environnement
Ce projet ne fournit ni conteneur de développement, ni Dockerfile, ni guide de contribution : l'installation est à votre charge. Commencez par son README, et consultez notre guide de la première contribution pour les étapes générales.
Par où commencer
- Lisez l'issue en entier, puis le guide de contribution du projet.
- Signalez en commentaire que vous la prenez — cela évite que deux personnes fassent le même travail.
- Forkez le dépôt et travaillez sur une branche.
- Ouvrez une pull request qui référence le numéro de l'issue.
Autres issues de apache/iceberg-cpp
-
bug: ReferenceVisitor::GetReferencedFieldIds dereferences null on a bound COUNT(*)Peut-être pris @LuciferYang l’a pris il y a 11 jours. Ouverte
Difficulté 2/5 1-3 heures Accessibilité débutants 82/100
apache/iceberg-cpp#978 ·
Les mainteneurs répondent en général sous 2 jours
-
bug: InMemoryCatalog skips the namespace existence check in CreateTable and RegisterTablePeut-être pris @LuciferYang l’a pris il y a 11 jours. Ouverte
Difficulté 2/5 1-3 heures Accessibilité débutants 78/100
apache/iceberg-cpp#977 ·
Les mainteneurs répondent en général sous 2 jours
-
Difficulté 4/5 3-5 jours Accessibilité débutants 35/100
apache/iceberg-cpp#1000 · 1 commentaire · 1 réaction ·
Les mainteneurs répondent en général sous 2 jours
-
Support Iceberg table encryption (Java-compatible)Peut-être pris Une pull request liée à cette issue est ouverte ou déjà fusionnée. Ouverte
Difficulté 5/5 Plus d'une semaine Accessibilité débutants 8/100
apache/iceberg-cpp#988 · 1 commentaire ·
Les mainteneurs répondent en général sous 2 jours
-
Difficulté 5/5 Plus d'une semaine Accessibilité débutants 20/100
apache/iceberg-cpp#959 · 1 commentaire ·
Les mainteneurs répondent en général sous 2 jours
Toutes les issues de apache/iceberg-cpp
Issues similaires
-
Difficulté 1/5 Moins d'une heure Accessibilité débutants 86/100
AcademySoftwareFoundation/Imath#611 ·
Les mainteneurs répondent en général sous 1 jour
-
bug intrinsic low priority rt_lib
Difficulté 2/5 1-3 heures Accessibilité débutants 78/100
Les mainteneurs répondent en général sous 1 jour
-
The Outdated Version prompt shows the release tag as unescaped HTMLPeut-être pris @on8st l’a pris aujourd’hui. Ouvertebug good first issue GUI maintainer-review priority: low
Difficulté 2/5 1-3 heures Accessibilité débutants 76/100
aethersdr/AetherSDR#6405 · 2 commentaires ·
Les mainteneurs répondent en général sous 1 jour
-
Difficulté 2/5 1-3 heures Accessibilité débutants 68/100
bitcoin-core/gui-qml#1003 ·
Les mainteneurs répondent en général sous 3 jours
-
fails to compile with Qt6Ouverte
Difficulté 2/5 1-3 heures Accessibilité débutants 75/100