Add micro benchmarks for unified query layer#5043
Conversation
dai-chen
added
the
infrastructure
Signed-off-by: Chen Dai <daichen@amazon.com>
Signed-off-by: Chen Dai <daichen@amazon.com>
dai-chen
force-pushed
the
add-microbenchmark-for-unified-query-api
branch
from
7988b7b to
8e50ba5
Compare
Signed-off-by: Chen Dai <daichen@amazon.com>
dai-chen
changed the title
Signed-off-by: Chen Dai <daichen@amazon.com>
dai-chen
requested review from
GumpacG,
LantaoJin,
MaxKsyunz,
Swiddis,
YANG-DB,
Yury-Fridlyand,
acarbonetto,
anirudha,
derek-ho,
forestmvey,
joshuali925,
kavithacm,
mengweieric,
noCharger,
penghuo,
ps48,
seankao-az,
vamsimanohar and
ykmr1224
as code owners
dai-chen
requested review from
RyanL1997,
qianheng-aws and
yuancu
as code owners
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@benchmarks/src/jmh/java/org/opensearch/sql/api/UnifiedQueryBenchmark.java`:
- Around line 81-86: The benchmark method compileQuery() returns a
PreparedStatement from compiler.compile(plan) but never closes it per iteration;
change the benchmark to close the PreparedStatement each iteration (either close
the returned PreparedStatement before returning or switch to void and close
within the method) so instances from compiler.compile(...) are released; update
the compileQuery() method to obtain RelNode plan = planner.plan(query), call
PreparedStatement stmt = compiler.compile(plan), ensure stmt.close() is invoked
in a finally block (or use try-with-resources) to guarantee per-iteration
cleanup.
🧹 Nitpick comments (1)
benchmarks/src/jmh/java/org/opensearch/sql/api/UnifiedFunctionBenchmark.java (1)
113-118: Consider adding a meaningful message toorElseThrow().If the function is not found in the repository, the exception will have no context about which function failed to load.
Suggested improvement
UnifiedFunction loadFunction(UnifiedFunctionRepository repository) { return repository .loadFunction(name()) .map(desc -> desc.getBuilder().build(inputTypes)) - .orElseThrow(); + .orElseThrow(() -> new IllegalStateException("Function not found: " + name())); }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
api/src/testFixtures/java/org/opensearch/sql/api/UnifiedQueryTestBase.javabenchmarks/README.mdbenchmarks/build.gradlebenchmarks/src/jmh/java/org/opensearch/sql/api/UnifiedFunctionBenchmark.javabenchmarks/src/jmh/java/org/opensearch/sql/api/UnifiedQueryBenchmark.java
🧰 Additional context used
📓 Path-based instructions (1)
**/*.java
📄 CodeRabbit inference engine (.rules/REVIEW_GUIDELINES.md)
**/*.java: UsePascalCasefor class names (e.g.,QueryExecutor)
UsecamelCasefor method and variable names (e.g.,executeQuery)
UseUPPER_SNAKE_CASEfor constants (e.g.,MAX_RETRY_COUNT)
Keep methods under 20 lines with single responsibility
All public classes and methods must have proper JavaDoc
Use specific exception types with meaningful messages for error handling
PreferOptional<T>for nullable returns in Java
Avoid unnecessary object creation in loops
UseStringBuilderfor string concatenation in loops
Validate all user inputs, especially queries
Sanitize data before logging to prevent injection attacks
Use try-with-resources for proper resource cleanup in Java
Maintain Java 11 compatibility when possible for OpenSearch 2.x
Document Calcite-specific workarounds in code
Files:
benchmarks/src/jmh/java/org/opensearch/sql/api/UnifiedQueryBenchmark.javabenchmarks/src/jmh/java/org/opensearch/sql/api/UnifiedFunctionBenchmark.java
⚙️ CodeRabbit configuration file
**/*.java: - Flag methods >50 lines as potentially too complex - suggest refactoring
- Flag classes >500 lines as needing organization review
- Check for dead code, unused imports, and unused variables
- Identify code reuse opportunities across similar implementations
- Assess holistic maintainability - is code easy to understand and modify?
- Flag code that appears AI-generated without sufficient human review
- Verify Java naming conventions (PascalCase for classes, camelCase for methods/variables)
- Check for proper JavaDoc on public classes and methods
- Flag redundant comments that restate obvious code
- Ensure proper error handling with specific exception types
- Check for Optional usage instead of null returns
- Validate proper use of try-with-resources for resource management
Files:
benchmarks/src/jmh/java/org/opensearch/sql/api/UnifiedQueryBenchmark.javabenchmarks/src/jmh/java/org/opensearch/sql/api/UnifiedFunctionBenchmark.java
🧠 Learnings (1)
📚 Learning: 2025-12-02T17:27:55.938Z
Learnt from: CR
Repo: opensearch-project/sql PR: 0
File: .rules/REVIEW_GUIDELINES.md:0-0
Timestamp: 2025-12-02T17:27:55.938Z
Learning: Test SQL generation and optimization paths for Calcite integration changes
Applied to files:
benchmarks/src/jmh/java/org/opensearch/sql/api/UnifiedQueryBenchmark.java
🧬 Code graph analysis (2)
benchmarks/src/jmh/java/org/opensearch/sql/api/UnifiedQueryBenchmark.java (1)
api/src/main/java/org/opensearch/sql/api/compiler/UnifiedQueryCompiler.java (1)
UnifiedQueryCompiler(24-71)
benchmarks/src/jmh/java/org/opensearch/sql/api/UnifiedFunctionBenchmark.java (1)
benchmarks/src/jmh/java/org/opensearch/sql/api/UnifiedQueryBenchmark.java (1)
Warmup(32-87)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (16)
- GitHub Check: build-linux (25, integration)
- GitHub Check: build-linux (21, unit)
- GitHub Check: build-linux (21, integration)
- GitHub Check: build-linux (25, unit)
- GitHub Check: CodeQL-Scan (java)
- GitHub Check: security-it-windows-macos (macos-14, 25)
- GitHub Check: security-it-windows-macos (windows-latest, 21)
- GitHub Check: build-windows-macos (macos-14, 21, doc)
- GitHub Check: build-windows-macos (macos-14, 25, integration)
- GitHub Check: build-windows-macos (macos-14, 21, unit)
- GitHub Check: build-windows-macos (macos-14, 25, unit)
- GitHub Check: build-windows-macos (macos-14, 21, integration)
- GitHub Check: build-windows-macos (windows-latest, 25, -PbuildPlatform=windows, unit)
- GitHub Check: build-windows-macos (windows-latest, 21, -PbuildPlatform=windows, integration)
- GitHub Check: build-windows-macos (windows-latest, 21, -PbuildPlatform=windows, unit)
- GitHub Check: build-windows-macos (windows-latest, 25, -PbuildPlatform=windows, integration)
🔇 Additional comments (9)
benchmarks/build.gradle (2)
36-39: JMH includes configuration looks good.The configuration correctly reads from the project property and defaults to an empty list (run all benchmarks). This aligns with the README documentation.
19-20: Dependency additions are appropriate.The
apimodule dependency andtestFixturesforjmhImplementationcorrectly expose theUnifiedQueryTestBaseand other shared types needed by the new benchmarks.benchmarks/README.md (1)
13-24: Clear and practical documentation.The updated instructions are well-structured, showing both running all benchmarks and filtering by class/method patterns. The examples align with the
jmh.includesconfiguration inbuild.gradle.benchmarks/src/jmh/java/org/opensearch/sql/api/UnifiedFunctionBenchmark.java (3)
31-48: Well-structured benchmark class with proper JavaDoc.The class is properly documented with JMH annotations matching
UnifiedQueryBenchmarkfor consistency. The benchmark categories (json, math, conditional, collection, string) are clearly represented.
85-92: Correct use of@OperationsPerInvocationandBlackhole.The
evalFunctionbenchmark properly uses@OperationsPerInvocation(OPS)to report per-operation timings andBlackhole.consume()to prevent dead code elimination.
94-127: Clean enum design for benchmark specifications.The
BenchmarkSpecenum effectively encapsulates function test cases with their input types and sample values. The helper methodsinputTypes()andsampleInputs()improve readability.benchmarks/src/jmh/java/org/opensearch/sql/api/UnifiedQueryBenchmark.java (3)
28-38: Well-structured benchmark class with consistent JMH configuration.The class properly documents its purpose and uses consistent JMH annotations matching
UnifiedFunctionBenchmark. The benchmark methods are focused and return results to prevent dead code elimination.
40-48: Good coverage of representative PPL query patterns.The query parameters cover essential operations: simple source, filtering (where), aggregation (stats), sorting, and combined pipelines. This provides meaningful baseline metrics.
68-79: Benchmark methods correctly return values.Both
planQuery()andtranspileToSql()return their results, preventing JIT dead code elimination without needingBlackhole. This is the recommended JMH pattern when the result is a reference type.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
Signed-off-by: Chen Dai <daichen@amazon.com>
* Add microbenchmark for unified query APIs Signed-off-by: Chen Dai <daichen@amazon.com> * Add benchmark for unified function Signed-off-by: Chen Dai <daichen@amazon.com> * Refactor javadoc and helper method Signed-off-by: Chen Dai <daichen@amazon.com> * Fix spotless check failure Signed-off-by: Chen Dai <daichen@amazon.com> * Address CodeRabit comment Signed-off-by: Chen Dai <daichen@amazon.com> --------- Signed-off-by: Chen Dai <daichen@amazon.com> (cherry picked from commit cee7f6b)
* Add microbenchmark for unified query APIs * Add benchmark for unified function * Refactor javadoc and helper method * Fix spotless check failure * Address CodeRabit comment --------- (cherry picked from commit cee7f6b) Signed-off-by: Chen Dai <daichen@amazon.com>
4 participants
Description
Add JMH microbenchmarks for the unified query layer to establish baseline performance metrics for query planning, transpilation, compilation, and unified function evaluation.
Changes
UnifiedQueryBenchmark: benchmarks for query planning, SQL transpilation, and compilation pipelines across common PPL query patterns.UnifiedFunctionBenchmark: benchmarks for function loading and evaluation covering representative functions from different categories (json, math, conditional, collection, string).Sample Test Results
Related Issues
Resolves partially #4782
Check List
--signoffor-s.By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.