Speed up translation phase without losing #128's execution-time and SQL-size gains
Nobody has claimed this yet.
- Dominant language
- Python
- Stars
- 120
- Forks
- 9
- Avg merge
- 11d 6m
- Merged PRs (30d)
- 2
Description
Goal
Investigate how to speed up the translation phase, without giving back the execution-time and SQL-size improvements #128 brought.
Done: identify why translation got significantly slower after #128, and reduce translate-phase time as long as execution time and SQL size stay at least as good as they are today. Add benchmark coverage that reports translation time and execution time as two separate numbers, so this tradeoff is visible.
Why
Commit 49643036716b9b719a8cfe02520803bcb8057a03 (PR: preserve oversized expressions automatically #128) traded translation time for faster query execution and smaller generated SQL. Execution time and SQL size are the most important things for Orbital, since the generated SQL is what actually runs on the target database, so this was a good trade on those two axes. But it also made the translate phase (building and optimizing the query before it is ever run) significantly slower.
Timing each phase separately (export_sql, orbital.translate, ibis's own compile step, and raw DuckDB execution of the generated SQL) on the pipeline.boosted.tree.binary.classifier example, before vs after #128:
| Phase | Before (#127, 5e33b43) | After (#128, 4964303) | Δ |
|---|---|---|---|
| Generated SQL size | ~203KB | ~184KB | smaller |
export_sql (translate + optimize -> SQL text) |
5.6s | 27.7s | ~5x slower |
orbital.translate (build ibis expression) |
1.2s | 18.0s | ~14x slower |
ibis compile (expression -> SQL, inside con.execute) |
2.0s | 7.2s | ~3.6x slower |
DuckDB executing the generated SQL (raw_sql(...).fetchall()) |
0.89s | 0.27s | faster |
A cProfile run on export_sql after #128 attributes most of the added translate time to preserve() calling ibis's .mutate() repeatedly (225 calls for this pipeline); each call triggers ibis's dereference / _fill_substitution_mappings rewrite over the whole growing expression tree, so the cost compounds as the tree grows.
Benchmarks today measure the whole example run as one number, and an upcoming benchmark change would measure only the execution phase. Neither would surface this translate/execution tradeoff, since the two move in opposite directions here. It might be useful for benchmarks to time translation and execution separately so both sides of this tradeoff stay visible going forward.
References
- PR: preserve oversized expressions automatically #128
- Issue: Materialize layers (preserve()) on the MatMul path to fix SQL blowup for sklearn MLPs #115 (the SQL-size/execution problem #128 addressed)
- src/orbital/translation/translator.py (
preserve) - src/orbital/translation/optimizer.py (
preserve_referenced_outputs) - benchmarks/test_examples.py
- examples/pipeline_boosted_tree_binary_classifier.py
Contributor guide
No contributing guide indexed for this repository
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start with preserve in src/orbital/translation/translator.py and preserve_referenced_outputs in src/orbital/translation/optimizer.py, using the pipeline_boosted_tree_binary_classifier example and the reported cProfile behavior as context. Run benchmarks/test_examples.py and add separate translation and execution timing. Done means translation is faster while execution time and generated SQL size are no worse than the current results.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- python, scikit-learn, sql
- Domain
- backend, machine-learning, performance
- Issue type
- Refactor
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 48/100