JuliaData / TypedTables.jl

Simple, fast, column-based storage for data analysis in Julia
Other
145 stars 25 forks source link

Change __getindex_dims to work better with inference #100

Closed ianatol closed 1 year ago

ianatol commented 1 year ago

While working on the upcoming Core.Compiler change https://github.com/JuliaLang/julia/pull/44803, we discovered that inference had some problems seeing through _getindex_dims to do proper constant propagation. Currently this isn't a problem, but in the semi-concrete branch it caused a loss of accuracy. This change fixes that pre-emptively, and shouldn't have any effect otherwise.

CC: @Keno

codecov[bot] commented 1 year ago

Codecov Report

Merging #100 (bc1745a) into main (145b0fa) will decrease coverage by 0.22%. The diff coverage is 80.00%.

@@            Coverage Diff             @@
##             main     #100      +/-   ##
==========================================
- Coverage   69.63%   69.41%   -0.23%     
==========================================
  Files           7        7              
  Lines         909      909              
==========================================
- Hits          633      631       -2     
- Misses        276      278       +2     
Impacted Files Coverage Δ
src/FlexTable.jl 80.60% <80.00%> (ø)
src/properties.jl 78.94% <0.00%> (-0.76%) :arrow_down:
src/Table.jl 73.41% <0.00%> (-0.58%) :arrow_down:

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

ianatol commented 1 year ago

@andyferris I'm not sure if there will be a hugely noticeable performance benefit here, though there may be places where performance is more predictable. The problem that this compiler work aims to solve is improving performance of functions that are just ineligible for concrete evaluation, and generally smoothing the performance valley between concrete evaluation and normal constant propagation. If there are many such functions internal to typed table operations then it is possible that there may be a speed increase.