Add a minimal micro-benchmark suite to validate filtering hot-path optimizations
- Lenguaje dominante
- C++
- Estrellas
- 221
- Forks
- 124
- Merge medio
- 1 d 16 h
- PR fusionados (30 d)
- 21
Descripción
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:
```diff
- #define RETURN_IF_NOT_REFERENCE(expr) \
- if (auto ref = dynamic_cast(expr.get()); ref == nullptr) { \
- return kRowsMightNotMatch; \
- }
+ #define BIND_REFERENCE_OR_RETURN(ref, expr) \
+ const auto* ref = dynamic_cast((expr).get()); \
+ if (ref == nullptr) { \
+ return kRowsMightNotMatch; \
+ }
Result IsNull(const std::shared_ptr& 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:
1. **How** should we add it — Google Benchmark fetched the same way googletest already is, behind an off-by-default CMake option?
2. **Where** should it live — a top-level `benchmark/`, or co-located under `src/iceberg/**/`?
I'm happy to put up a draft PR for a minimal suite + the filtering benchmark above once there's agreement on direction.
Guía de contribución
No hay ninguna guía de contribución indexada para este repositorio
Línea de trabajo
Empieza revisando la configuración existente de CMake y GoogleTest y, a continuación, sigue la ruta de filtrado bajo src/iceberg/**/ y sus evaluadores de métricas. El trabajo estará terminado cuando el proyecto tenga una suite de benchmarks mínima acordada, una opción de compilación desactivada de forma predeterminada y un benchmark que mida el paso de filtrado de forma aislada.
Escrito por el modelo de indexación a partir del texto del issue.
Evaluación
- Stack tecnológico
- cpp
- Área
- build-system, performance, tooling
- Tipo de issue
- Nueva funcionalidad
- Dificultad
- 4/5
- Tiempo estimado
- 3-5 días
- Estado de actividad
- Tranquilo
- Claridad
- Necesita aclaración
- Aptitud para principiantes
- 35/100