Unify all runtime env var access to functions wrapping LazyLock#7468
Unify all runtime env var access to functions wrapping LazyLock#7468
Conversation
Signed-off-by: Adam Gutglick <adam@spiraldb.com>
Merging this PR will degrade performance by 24.54%
Performance Changes
Comparing Footnotes
|
Signed-off-by: Adam Gutglick <adam@spiraldb.com>
Polar Signals Profiling ResultsLatest Run
Previous Runs (1)
Powered by Polar Signals Cloud |
Benchmarks: PolarSignals ProfilingVortex (geomean): 1.036x ➖ datafusion / vortex-file-compressed (1.036x ➖, 0↑ 2↓)
|
File Sizes: PolarSignals ProfilingNo file size changes detected. |
Benchmarks: TPC-H SF=1 on NVMEVerdict: No clear signal (low confidence) datafusion / vortex-file-compressed (0.992x ➖, 0↑ 0↓)
datafusion / vortex-compact (1.000x ➖, 0↑ 0↓)
datafusion / parquet (0.986x ➖, 1↑ 0↓)
datafusion / arrow (0.984x ➖, 1↑ 0↓)
duckdb / vortex-file-compressed (0.989x ➖, 0↑ 0↓)
duckdb / vortex-compact (0.993x ➖, 0↑ 0↓)
duckdb / parquet (0.997x ➖, 2↑ 2↓)
duckdb / duckdb (0.998x ➖, 0↑ 0↓)
Full attributed analysis
|
Benchmarks: FineWeb NVMeVerdict: No clear signal (low confidence) datafusion / vortex-file-compressed (0.988x ➖, 1↑ 0↓)
datafusion / vortex-compact (1.010x ➖, 0↑ 0↓)
datafusion / parquet (1.000x ➖, 0↑ 0↓)
duckdb / vortex-file-compressed (1.009x ➖, 1↑ 1↓)
duckdb / vortex-compact (1.012x ➖, 1↑ 0↓)
duckdb / parquet (1.019x ➖, 0↑ 0↓)
Full attributed analysis
|
File Sizes: FineWeb NVMeNo file size changes detected. |
File Sizes: TPC-H SF=1 on NVMENo file size changes detected. |
Benchmarks: TPC-DS SF=1 on NVMEVerdict: No clear signal (low confidence) datafusion / vortex-file-compressed (0.999x ➖, 0↑ 0↓)
datafusion / vortex-compact (0.996x ➖, 0↑ 0↓)
datafusion / parquet (1.006x ➖, 0↑ 2↓)
duckdb / vortex-file-compressed (1.013x ➖, 1↑ 1↓)
duckdb / vortex-compact (1.006x ➖, 3↑ 2↓)
duckdb / parquet (1.005x ➖, 1↑ 2↓)
duckdb / duckdb (0.999x ➖, 1↑ 1↓)
Full attributed analysis
|
File Sizes: TPC-DS SF=1 on NVMENo file size changes detected. |
Benchmarks: FineWeb S3Verdict: No clear signal (environment too noisy confidence) datafusion / vortex-file-compressed (1.099x ➖, 0↑ 1↓)
datafusion / vortex-compact (1.101x ➖, 0↑ 0↓)
datafusion / parquet (1.134x ➖, 0↑ 2↓)
duckdb / vortex-file-compressed (1.301x ❌, 0↑ 5↓)
duckdb / vortex-compact (1.188x ➖, 0↑ 2↓)
duckdb / parquet (1.062x ➖, 0↑ 0↓)
Full attributed analysis
|
Benchmarks: Statistical and Population GeneticsVerdict: No clear signal (low confidence) duckdb / vortex-file-compressed (0.986x ➖, 0↑ 0↓)
duckdb / vortex-compact (0.980x ➖, 0↑ 0↓)
duckdb / parquet (0.966x ➖, 0↑ 0↓)
Full attributed analysis
|
File Sizes: Statistical and Population GeneticsNo file size changes detected. |
Benchmarks: TPC-H SF=1 on S3Verdict: No clear signal (environment too noisy confidence) datafusion / vortex-file-compressed (1.104x ➖, 0↑ 2↓)
datafusion / vortex-compact (1.052x ➖, 0↑ 1↓)
datafusion / parquet (1.043x ➖, 0↑ 1↓)
duckdb / vortex-file-compressed (1.059x ➖, 0↑ 0↓)
duckdb / vortex-compact (1.066x ➖, 0↑ 0↓)
duckdb / parquet (1.045x ➖, 0↑ 1↓)
Full attributed analysis
|
Benchmarks: Clickbench on NVMEVerdict: No clear signal (low confidence) datafusion / vortex-file-compressed (1.131x ❌, 0↑ 28↓)
datafusion / parquet (1.094x ➖, 0↑ 21↓)
duckdb / vortex-file-compressed (1.076x ➖, 0↑ 10↓)
duckdb / parquet (1.053x ➖, 0↑ 3↓)
duckdb / duckdb (1.034x ➖, 0↑ 4↓)
Full attributed analysis
|
File Sizes: Clickbench on NVMEFile Size Changes (1 files changed, -0.0% overall, 0↑ 1↓)
Totals:
|
Benchmarks: Random AccessVortex (geomean): 0.855x ✅ unknown / unknown (0.960x ➖, 8↑ 3↓)
|
Benchmarks: CompressionVortex (geomean): 1.011x ➖ unknown / unknown (1.003x ➖, 0↑ 1↓)
|
Benchmarks: TPC-H SF=10 on S3Verdict: No clear signal (environment too noisy confidence) datafusion / vortex-file-compressed (1.073x ➖, 0↑ 1↓)
datafusion / vortex-compact (1.077x ➖, 0↑ 2↓)
datafusion / parquet (1.095x ➖, 0↑ 4↓)
duckdb / vortex-file-compressed (1.109x ➖, 0↑ 1↓)
duckdb / vortex-compact (1.085x ➖, 0↑ 1↓)
duckdb / parquet (1.098x ➖, 0↑ 0↓)
Full attributed analysis
|
Benchmarks: TPC-H SF=10 on NVMEVerdict: No clear signal (low confidence) datafusion / vortex-file-compressed (1.027x ➖, 0↑ 0↓)
datafusion / vortex-compact (1.020x ➖, 0↑ 0↓)
datafusion / parquet (1.008x ➖, 0↑ 0↓)
datafusion / arrow (1.013x ➖, 0↑ 0↓)
duckdb / vortex-file-compressed (1.010x ➖, 0↑ 0↓)
duckdb / vortex-compact (1.000x ➖, 0↑ 0↓)
duckdb / parquet (1.003x ➖, 0↑ 0↓)
duckdb / duckdb (1.002x ➖, 0↑ 0↓)
Full attributed analysis
|
File Sizes: TPC-H SF=10 on NVMENo file size changes detected. |
|
Shall we unify all the env vars into a single unstable one? |
Summary
Env variable access isn't free, this is a fairly common pattern we already used in a couple of places, this PR just makes it consistent across the codebase.
API Changes
Replaced the experimental patches from a public
LazyLockto a function. I think this should've been a feature an not a runtime thing, but that's a different change.I've also removed the
EnvVarGuardfromvortex-utils, it was only used in one place and I've just replaced it withtemp-envthat we already use elsewhere.Testing
For the few tests that toggled env variables, I've changed them to only run on nextest, relying on it process isolation.