Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix performance of Containers.rowtable #3410

Merged
merged 2 commits into from Jun 7, 2023
Merged

Conversation

odow
Copy link
Member

@odow odow commented Jun 6, 2023

Closes #3404.

Julia 1.6

julia> using JuMP.Containers

julia> Containers.@container(x[i=1:100, j=1:50], i + j);

julia> function foo(x, header)
           names = tuple(header...)
           return [NamedTuple{names}((args..., x[i])) for (i, args) in Containers._rows(x)]
       end
foo (generic function with 1 method)

julia> function bar(x, header)
           elements = [(args..., x[i]) for (i, args) in Containers._rows(x)]
           return NamedTuple{tuple(header...)}.(elements)
       end
bar (generic function with 1 method)

julia> @assert foo(x, [:i, :j, :k]) == bar(x, [:i, :j, :k])

julia> @time foo(x, [:i, :j, :k]);
  0.006314 seconds (30.02 k allocations: 1.597 MiB)

julia> @time bar(x, [:i, :j, :k]);
  0.000154 seconds (23 allocations: 502.297 KiB)

Julia 1.9

julia> using JuMP.Containers

julia> Containers.@container(x[i=1:100, j=1:50], i + j);

julia> function foo(x, header)
           names = tuple(header...)
           return [NamedTuple{names}((args..., x[i])) for (i, args) in Containers._rows(x)]
       end
foo (generic function with 1 method)

julia> function bar(x, header)
           elements = [(args..., x[i]) for (i, args) in Containers._rows(x)]
           return NamedTuple{tuple(header...)}.(elements)
       end
bar (generic function with 1 method)

julia> @assert foo(x, [:i, :j, :k]) == bar(x, [:i, :j, :k])

julia> @time foo(x, [:i, :j, :k]);
  0.378728 seconds (140.02 k allocations: 5.545 MiB)

julia> @time bar(x, [:i, :j, :k]);
  0.000079 seconds (18 allocations: 482.516 KiB)

I don't understand the reason for the regression in 1.9. But this is better regardless.

src/Containers/tables.jl Outdated Show resolved Hide resolved
@codecov
Copy link

codecov bot commented Jun 6, 2023

Codecov Report

Patch coverage: 100.00% and no project coverage change.

Comparison is base (19ab5d6) 98.05% compared to head (763d625) 98.05%.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #3410   +/-   ##
=======================================
  Coverage   98.05%   98.05%           
=======================================
  Files          34       34           
  Lines        4926     4926           
=======================================
  Hits         4830     4830           
  Misses         96       96           
Impacted Files Coverage Δ
src/Containers/tables.jl 100.00% <100.00%> (ø)

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@odow
Copy link
Member Author

odow commented Jun 7, 2023

Just to close the loop on the original benchmark:

Previously

julia> @btime JuMP.Containers.rowtable($value, $x, header=$header);
  371.580 ms (210016 allocations: 7.07 MiB)

julia> @btime test($value, $x, header=$header);
  6.915 ms (105016 allocations: 3.03 MiB)

julia> @btime JuMP.Containers.rowtable($value, $y, header=$header);
  187.218 ms (105010 allocations: 3.41 MiB)

julia> @btime test($value, $y, header=$header);
  3.433 ms (52510 allocations: 1.39 MiB)

This PR

julia> @btime JuMP.Containers.rowtable(value, $x, header=$header);
  2.349 ms (55025 allocations: 1.46 MiB)

julia> @btime test(value, $x, header=$header);
  6.906 ms (105016 allocations: 3.03 MiB)

julia> @btime JuMP.Containers.rowtable(value, $y, header=$header);
  1.211 ms (27518 allocations: 625.70 KiB)

julia> @btime test(value, $y, header=$header);
  3.354 ms (52510 allocations: 1.39 MiB)

@odow odow merged commit 527e6f5 into master Jun 7, 2023
12 checks passed
@odow odow deleted the od/rowtable-performance branch June 7, 2023 03:21
hellemo added a commit to sintefore/SparseVariables.jl that referenced this pull request Jun 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Containers.rowtable is slow on Julia 1.9
1 participant