Skip to content

Memoize Coeffs #73

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

Merged
merged 2 commits into from
Apr 15, 2020
Merged

Memoize Coeffs #73

merged 2 commits into from
Apr 15, 2020

Conversation

oxinabox
Copy link
Member

Closes #72.
Now we are faster than we were even on 0.9.2 before I made it slow.
This does part of #61 in particular part of this comment
#61 (comment)

Looks like the cause was #65.
Am going to build a fix shortly

First two timings are as per #72 (comment)

v0.9.5

44.084013 seconds (80.42 M allocations: 4.397 GiB, 3.80% gc time)
29.377420 seconds (28.23 M allocations: 1.830 GiB, 1.86% gc time)
27.364521 seconds (28.23 M allocations: 1.830 GiB, 1.76% gc time)
27.871987 seconds (28.23 M allocations: 1.830 GiB, 1.79% gc time)

v0.9.2

18.984412 seconds (61.60 M allocations: 3.236 GiB, 6.57% gc time)
1.072190 seconds (10.23 M allocations: 737.725 MiB, 10.14% gc time)
1.090849 seconds (10.23 M allocations: 737.725 MiB, 9.55% gc time)
1.072175 seconds (10.23 M allocations: 737.725 MiB, 8.74% gc time)

With this PR

10.143730 seconds (40.32 M allocations: 1.860 GiB, 8.13% gc time)
0.634321 seconds (7.34 M allocations: 243.141 MiB, 7.87% gc time)
0.631055 seconds (7.34 M allocations: 243.141 MiB, 8.24% gc time)
0.661258 seconds (7.34 M allocations: 243.141 MiB, 10.92% gc time)

Using timing from #61

using BenchmarkTools
using FiniteDifferences

const _fdm = central_fdm(2,1);
const _fdm5 = central_fdm(5,1);

const xs = collect(1:0.1:200);
f(x) = sum(sin, x);
@btime grad($_fdm, $f, $xs);
@btime grad($_fdm5, $f, $xs);

We get:

v0.9.5

julia> @btime grad($_fdm, $f, $xs);
  226.006 ms (195137 allocations: 9.54 MiB)

julia> @btime grad($_fdm5, $f, $xs);
  744.445 ms (665012 allocations: 23.76 MiB)

v0.9.2

julia> @btime grad($_fdm, $f, $xs);
  221.743 ms (154789 allocations: 4.26 MiB)

julia> @btime grad($_fdm5, $f, $xs);
  440.230 ms (162753 allocations: 5.45 MiB)

With this PR

julia> @btime grad($_fdm, $f, $xs);
  199.677 ms (177218 allocations: 6.90 MiB)

julia> @btime grad($_fdm5, $f, $xs);
  404.316 ms (230974 allocations: 11.58 MiB)

Explain

I think the main reason why this gives much more improvement in the test case from #72 than from #61, is that the #61 benchmark declares the fdm objects outside the the hot-loop.
And that is where _coeffs are calculated.
Except I think they are also calculated when adapt is triggered.
So this helps more there for the _fdm5 case.

Copy link
Contributor

@nickrobinson251 nickrobinson251 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work!

@oxinabox oxinabox merged commit ccb9c75 into master Apr 15, 2020
@oxinabox oxinabox deleted the ox/memoize_coeffs branch April 16, 2020 09:12
x = zeros(Rational{Int128}, p)
x[q + 1] = factorial(q)
return Float64.(C \ x)
return get!(_COEFFS_CACHE, (grid, p, q)) do

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it worth thinking about thread safety here?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could do, but I don't think it is worth it.
There is much more thread unsafe stuff in this package that this.
(Mostly the history tracking stuff).

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.

Huge detriment in computational performance
3 participants