Remove duckdbfs#8468
Conversation
fa938c8 to
e41bcc7
Compare
Merging this PR will not alter performance
|
| Mode | Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|---|
| ❌ | Simulation | decompress_rd[f64, (10000, 0.01)] |
108.7 µs | 139.1 µs | -21.89% |
| ❌ | Simulation | decompress_rd[f64, (10000, 0.1)] |
109 µs | 139.5 µs | -21.85% |
| ❌ | Simulation | decompress_rd[f64, (10000, 0.0)] |
108.7 µs | 139.1 µs | -21.83% |
| ❌ | Simulation | decompress_rd[f32, (100000, 0.0)] |
496 µs | 583.8 µs | -15.05% |
| ❌ | Simulation | decompress_rd[f32, (10000, 0.1)] |
78.1 µs | 91.2 µs | -14.43% |
| ❌ | Simulation | decompress_rd[f32, (10000, 0.01)] |
78.1 µs | 91 µs | -14.2% |
| ❌ | Simulation | decompress_rd[f32, (10000, 0.0)] |
78.5 µs | 91.2 µs | -13.91% |
| ⚡ | Simulation | chunked_varbinview_opt_canonical_into[(1000, 10)] |
206.8 µs | 170.2 µs | +21.45% |
| ⚡ | Simulation | bitwise_not_vortex_buffer_mut[128] |
215.3 ns | 186.1 ns | +15.67% |
| ⚡ | Simulation | chunked_varbinview_into_canonical[(100, 100)] |
307.1 µs | 272.5 µs | +12.71% |
| ⚡ | Simulation | bitwise_not_vortex_buffer_mut[1024] |
275.6 ns | 246.4 ns | +11.84% |
Tip
Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.
Comparing myrrc/remove-duckdbfs (c02a5ba) with develop (6a3fb19)
43a74b1 to
5e2a617
Compare
Signed-off-by: Mikhail Kot <mikhail@spiraldb.com>
5e2a617 to
c02a5ba
Compare
0ax1
left a comment
There was a problem hiding this comment.
I'm actually in favor of dropping this. The perf doesn't match and as we never exercise or bench the code the level of maintenance is low.
Polar Signals Profiling ResultsLatest Run
Powered by Polar Signals Cloud |
Benchmarks: FineWeb NVMe (base)Verdict: No clear signal (low confidence) How to read Verdict and Engines
datafusion / vortex-file-compressed (1.024x ➖, 0↑ 2↓)
datafusion / parquet (1.017x ➖, 0↑ 0↓)
duckdb / vortex-file-compressed (0.886x ✅, 5↑ 0↓)
duckdb / parquet (1.035x ➖, 0↑ 1↓)
File Size Changes (3 files changed, -46.3% overall, 0↑ 3↓)
Totals:
|
Benchmarks: PolarSignals Profiling (base)Vortex (geomean): 1.110x ❌ How to read Verdict and Engines
datafusion / vortex-file-compressed (1.110x ❌, 0↑ 6↓)
No file size changes detected. |
Benchmarks: TPC-H SF=1 on NVME (base)Verdict: No clear signal (low confidence) How to read Verdict and Engines
datafusion / vortex-file-compressed (0.933x ➖, 1↑ 0↓)
datafusion / parquet (0.984x ➖, 0↑ 1↓)
datafusion / arrow (0.912x ➖, 9↑ 0↓)
duckdb / vortex-file-compressed (0.867x ✅, 20↑ 0↓)
duckdb / parquet (0.936x ➖, 5↑ 0↓)
File Size Changes (17 files changed, -44.5% overall, 2↑ 15↓)
Totals:
|
Benchmarks: TPC-DS SF=1 on NVME (base)Verdict: No clear signal (low confidence) How to read Verdict and Engines
datafusion / vortex-file-compressed (0.965x ➖, 4↑ 0↓)
datafusion / parquet (0.955x ➖, 12↑ 0↓)
duckdb / vortex-file-compressed (1.001x ➖, 0↑ 0↓)
duckdb / parquet (0.996x ➖, 0↑ 1↓)
File Size Changes (31 files changed, -43.5% overall, 4↑ 27↓)
Totals:
|
Benchmarks: FineWeb S3 (base)Verdict: No clear signal (low confidence) How to read Verdict and Engines
datafusion / vortex-file-compressed (1.258x ➖, 0↑ 3↓)
datafusion / parquet (1.198x ➖, 0↑ 1↓)
duckdb / vortex-file-compressed (1.163x ➖, 0↑ 1↓)
duckdb / parquet (1.109x ➖, 0↑ 0↓)
|
Benchmarks: Statistical and Population Genetics (base)Verdict: No clear signal (low confidence) How to read Verdict and Engines
duckdb / vortex-file-compressed (1.005x ➖, 0↑ 0↓)
duckdb / parquet (0.985x ➖, 0↑ 0↓)
File Size Changes (3 files changed, -32.3% overall, 1↑ 2↓)
Totals:
|
Benchmarks: Clickbench on NVME (base)Verdict: No clear signal (low confidence) How to read Verdict and Engines
datafusion / vortex-file-compressed (1.033x ➖, 0↑ 1↓)
datafusion / parquet (1.012x ➖, 0↑ 1↓)
duckdb / vortex-file-compressed (1.009x ➖, 0↑ 0↓)
duckdb / parquet (1.006x ➖, 0↑ 2↓)
File Size Changes (201 files changed, -39.1% overall, 53↑ 148↓)
Totals:
|
Benchmarks: TPC-H SF=10 on NVME (base)Verdict: No clear signal (low confidence) How to read Verdict and Engines
datafusion / vortex-file-compressed (0.987x ➖, 0↑ 0↓)
datafusion / parquet (0.993x ➖, 0↑ 0↓)
datafusion / arrow (0.990x ➖, 1↑ 0↓)
duckdb / vortex-file-compressed (1.015x ➖, 0↑ 0↓)
duckdb / parquet (1.007x ➖, 0↑ 0↓)
File Size Changes (47 files changed, -44.5% overall, 8↑ 39↓)
Totals:
|
Benchmarks: TPC-H SF=1 on S3 (base)Verdict: No clear signal (environment too noisy confidence) How to read Verdict and Engines
datafusion / vortex-file-compressed (1.159x ➖, 0↑ 5↓)
datafusion / parquet (1.172x ➖, 0↑ 7↓)
duckdb / vortex-file-compressed (1.064x ➖, 0↑ 2↓)
duckdb / parquet (1.012x ➖, 0↑ 0↓)
|
DuckdbFS implementation for Vortex was introduced in #6198 as opt-out, but changed to opt-in in #6564 due to performance regressions.
There were multiple issues (#6709, #6565 #6685) associated with it which differ from vortex's file system behaviour.
It also requires additional dependencies CI which are a blocker for
#8486 since MacOS runner doesn't bundle openssl for x86_64 on arm, and builds fail.
As a long term goal, calling duckdb's blocking IO inside our event loop isn't the right abstraction. We want to allow duckdb to use its own IO outside vortex.
Duckdb fs is also not maintaned actively so we're removing it