MasonProtter / SumTypes.jl

An implementation of sum types in Julia
MIT License
104 stars 8 forks source link

Back to union storage #49

Closed MasonProtter closed 1 year ago

MasonProtter commented 1 year ago

This undoes the major overhaul of version 0.4 where we were doing computed storage. With this change, a bunch of things get a lot simpler, and we don't need to carry around extra parameters in the sum type.

While the computed storage did benefit some situations, julia's Union is also quite good. For all isbits data it should have the same performance profile as before, but for non-isbits data, there are some tradeoffs as to what ends up heap allocated and what doesn't, and I think that Union actually does a better job when you're modifying and interacting with the data since there's usually fewer pointers involved.

Fixes #30

codecov[bot] commented 1 year ago

Codecov Report

Merging #49 (a1e9a18) into master (f786349) will not change coverage. The diff coverage is 100.00%.

@@            Coverage Diff             @@
##            master       #49    +/-   ##
==========================================
  Coverage   100.00%   100.00%            
==========================================
  Files            4         3     -1     
  Lines          360       249   -111     
==========================================
- Hits           360       249   -111     
Impacted Files Coverage Δ
src/SumTypes.jl 100.00% <100.00%> (ø)
src/cases.jl 100.00% <100.00%> (ø)
src/sum_type.jl 100.00% <100.00%> (ø)
MasonProtter commented 1 year ago

@Krastanov, I know you have some code relying on SumTypes.jl for performance referenced in https://github.com/MasonProtter/SumTypes.jl/issues/28. Could I ask you to test out this PR and see if it causes any performance regressions (or improvements!) for you?

Krastanov commented 1 year ago

Yes, I should be able to do so today or tomorrow.

Krastanov commented 1 year ago

The only interesting benchmarks I have are with isbits objects, so maybe this is not too informative, but here are the results:

Tested on:

julia> versioninfo()
Julia Version 1.9.1
Commit 147bdf428cd (2023-06-07 08:27 UTC)
Platform Info:
  OS: Linux (x86_64-linux-gnu)
  CPU: 32 × AMD Ryzen 9 7950X 16-Core Processor
  WORD_SIZE: 64
  LIBM: libopenlibm
  LLVM: libLLVM-14.0.6 (ORCJIT, znver3)
  Threads: 32 on 32 virtual cores

If you want to double check things for yourself, these are the benchmarks that depend on sumtypes:

https://github.com/QuantumSavory/QuantumClifford.jl/blob/544ba079c55dea93946cadba2a3aa6cd5d72adca/benchmark/benchmarks.jl#L131-L138

MasonProtter commented 1 year ago

Well that’s quite encouraging news! I guess julia’s handling of enclosed unions here is a bit smarter than what I was doing and enabled some speedups.