Skip to content

feat!: read CHAR variables as String - #79

Merged
Beforerr merged 1 commit into
mainfrom
string-eltype
Oct 6, 2026
Merged

Beforerr merged 1 commit into
mainfrom
string-eltype

Conversation

@Beforerr

@Beforerr Beforerr commented Oct 6, 2026 •

Copy link
Copy Markdown
Member

Replaces StaticString{N,UInt8} with String as the eltype of CDF_CHAR/CDF_UCHAR variables. StaticString put the field width in the type, so each new width recompiled the DiskArrays/AbstractArray read path plus String/show.

Breaking: eltype(var) for char variables changes, and CommonDataFormat.StaticString is removed (nothing else in the org uses it).

Measurements (fresh process, min of 5)

scenario main this PR
using 0.063 s 0.066 s
one CDAWeb file: read data + labels, show labels 0.204 s 0.218 s
every char var in data/: Array, v[1], String., show 1.98 s 0.80 s
Array of every var in data/ 0.33 s 0.15 s
precompile time / pkgimage 3.1 s / 9.8 MB 3.2 s / 9.7 MB

First call per new width: Array(label) 10–14 ms → 0, label[1] 29 ms → 0.

Runtime: Array of an 8-element label goes from 151 ns to 229 ns. Synthetic 1e6 × 16-byte records: 16 ms to build Strings vs 0.26 ms memcpy (56 MB vs 16 MB). That only matters for very large record-varying string variables.

The single-file case is unchanged because its time goes to Base compiling show for Matrix{String} (~140–190 ms). Both versions pay that.

@codecov

codecov Bot commented Oct 6, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 92.85714% with 2 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
src/loading/variable.jl 92.30% 2 Missing ⚠️

📢 Thoughts on this report? Let us know!

@github-actions

github-actions Bot commented Oct 6, 2026 •

Copy link
Copy Markdown
Contributor

Benchmark Results (Julia v1)

Time benchmarks
main 79ebf46... main / 79ebf46...
elx/full_load 24.6 ± 1.9 μs 24.6 ± 1.9 μs 1 ± 0.11
elx/sum_array 2.67 ± 1.3 μs 2.56 ± 0.84 μs 1.04 ± 0.6
elx/sum_lazy 2.75 ± 0.51 μs 2.71 ± 0.64 μs 1.02 ± 0.31
elx/sum_var_access 1.93 ± 0.99 μs 1.97 ± 1.2 μs 0.982 ± 0.78
elx/var_access 0.11 ± 0.01 μs 0.111 ± 0.011 μs 0.991 ± 0.13
epochs/DateTime_to_TT2000 0.0653 ± 0.00033 ms 0.0621 ± 0.0003 ms 1.05 ± 0.0074
epochs/Epoch16_parse 0.31 ± 0.01 ms 0.31 ± 0.01 ms 1 ± 0.047
epochs/Epoch16_vs_DateTime 2.92 ± 0.01 μs 2.92 ± 0.01 μs 1 ± 0.0048
epochs/TT2000_parse 0.364 ± 0.01 ms 0.357 ± 0.01 ms 1.02 ± 0.041
epochs/TT2000_string 5.76 ± 1.5 ms 5.61 ± 1.6 ms 1.03 ± 0.4
epochs/TT2000_to_DateTime 0.118 ± 0.00064 ms 0.112 ± 0.00047 ms 1.06 ± 0.0073
epochs/TT2000_vs_DateTime 0.0587 ± 1e-05 ms 0.0525 ± 2e-05 ms 1.12 ± 0.00047
mms/full_load 17.4 ± 1.9 μs 17.3 ± 1.9 μs 1.01 ± 0.15
mms/sum_array 0.0948 ± 0.0015 s 0.0952 ± 0.0021 s 0.996 ± 0.027
mms/sum_slice 3.49 ± 0.065 ms 3.49 ± 0.41 ms 1 ± 0.12
mms/sum_var_access 3.49 ± 0.055 ms 3.5 ± 0.4 ms 0.996 ± 0.12
mms/var_access 0.07 ± 0.01 μs 0.07 ± 0.001 μs 1 ± 0.14
time_to_load 0.167 ± 0.00084 s 0.167 ± 0.0022 s 0.999 ± 0.014
Memory benchmarks
main 79ebf46... main / 79ebf46...
elx/full_load 0.217 k allocs: 24.8 kB 0.217 k allocs: 24.8 kB 1
elx/sum_array 12 allocs: 28 kB 12 allocs: 28 kB 1
elx/sum_lazy 12 allocs: 28 kB 12 allocs: 28 kB 1
elx/sum_var_access 14 allocs: 28.3 kB 14 allocs: 28.3 kB 1
elx/var_access 1 allocs: 0.234 kB 1 allocs: 0.234 kB 1
epochs/DateTime_to_TT2000 3 allocs: 0.0764 MB 3 allocs: 0.0764 MB 1
epochs/Epoch16_parse 3 allocs: 0.153 MB 3 allocs: 0.153 MB 1
epochs/Epoch16_vs_DateTime 0 allocs: 0 B 0 allocs: 0 B
epochs/TT2000_parse 3 allocs: 0.0764 MB 3 allocs: 0.0764 MB 1
epochs/TT2000_string 0.2 M allocs: 7.02 MB 0.2 M allocs: 7.02 MB 1
epochs/TT2000_to_DateTime 3 allocs: 0.0764 MB 3 allocs: 0.0764 MB 1
epochs/TT2000_vs_DateTime 0 allocs: 0 B 0 allocs: 0 B
mms/full_load 0.056 k allocs: 5.77 kB 0.056 k allocs: 5.77 kB 1
mms/sum_array 1.04 k allocs: 31.7 MB 1.04 k allocs: 31.7 MB 1
mms/sum_slice 0.069 k allocs: 1.29 MB 0.069 k allocs: 1.29 MB 1
mms/sum_var_access 0.069 k allocs: 1.29 MB 0.069 k allocs: 1.29 MB 1
mms/var_access 1 allocs: 0.219 kB 1 allocs: 0.219 kB 1
time_to_load 0.2 k allocs: 11.8 kB 0.204 k allocs: 11.9 kB 0.984

@Beforerr
Beforerr marked this pull request as ready for review October 6, 2026 14:23
StaticString{N} put the field width in the eltype, so every new width
recompiled the DiskArrays/AbstractArray paths (10-50 ms each, plus
String/show). CHAR data is small metadata (labels, units), so a
width-independent eltype wins.
@Beforerr
Beforerr merged commit f1b2137 into main Oct 6, 2026
5 checks passed
@Beforerr
Beforerr deleted the string-eltype branch October 6, 2026 14:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant