-
Notifications
You must be signed in to change notification settings - Fork 148
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
Stop precomputing chunk sizes #708
Conversation
This prevents constant propagation of the length of a static array. Alternative to #707
Co-authored-by: David Widmann <[email protected]>
To get this actually released, we should do this to the release-0.10 branch? |
Or at least backport it |
I ran the benchmark in #373, on the master branch and with this PR: masterjulia> @btime ForwardDiff.gradient(rosenbrock, x);
328.805 ns (3 allocations: 1.92 KiB)
julia> @btime ForwardDiff.gradient(rosenbrock, x);
331.407 ns (3 allocations: 1.92 KiB)
julia> @btime ForwardDiff.gradient(rosenbrock, x);
328.077 ns (3 allocations: 1.92 KiB) PR #708julia> @btime ForwardDiff.gradient(rosenbrock, x);
417.503 ns (3 allocations: 1.92 KiB)
julia> @btime ForwardDiff.gradient(rosenbrock, x);
406.040 ns (3 allocations: 1.92 KiB)
julia> @btime ForwardDiff.gradient(rosenbrock, x);
407.500 ns (3 allocations: 1.92 KiB) Setupjulia> versioninfo()
Julia Version 1.10.4
Commit 48d4fd48430 (2024-06-04 10:41 UTC)
Build Info:
Official https://julialang.org/ release
Platform Info:
OS: macOS (arm64-apple-darwin22.4.0)
CPU: 10 × Apple M2 Pro
WORD_SIZE: 64
LIBM: libopenlibm
LLVM: libLLVM-15.0.7 (ORCJIT, apple-m1)
Threads: 1 default, 0 interactive, 1 GC (on 6 virtual cores)
Environment:
JULIA_PKG_USE_CLI_GIT = true |
This was an easier way too see the regression: julia> const x = rand(10)
# master
julia> @btime ForwardDiff.Chunk($x)
3.792 ns (0 allocations: 0 bytes)
ForwardDiff.Chunk{10}()
# PR
julia> @btime ForwardDiff.Chunk($x)
55.894 ns (0 allocations: 0 bytes)
ForwardDiff.Chunk{10}() I made a new commit to try work around that. Could you try again @devmotion? |
I reran the benchmark in #373 with the latest commit: julia> @btime ForwardDiff.gradient(rosenbrock, x);
358.333 ns (3 allocations: 1.92 KiB)
julia> @btime ForwardDiff.gradient(rosenbrock, x);
346.847 ns (3 allocations: 1.92 KiB)
julia> @btime ForwardDiff.gradient(rosenbrock, x);
347.417 ns (3 allocations: 1.92 KiB) And that's what I get when I benchmark # master
julia> @btime ForwardDiff.Chunk($x)
3.458 ns (0 allocations: 0 bytes)
# PR
julia> @btime ForwardDiff.Chunk($x)
3.625 ns (0 allocations: 0 bytes) |
So would you say this is ok as it is now? |
Personally, I think the benefits (for type inference and static arrays) are worth the ~5% regression in these benchmarks, even more so since these benchmarks only focus on the construction of |
Should backport and release a new version |
This prevents constant propagation of the length of a static array. Alternative to #707 Co-authored-by: David Widmann <[email protected]>
This prevents constant propagation of the length of a static array. Alternative to #707 Co-authored-by: David Widmann <[email protected]>
This prevents constant propagation of the length of a static array. Alternative to #707 Co-authored-by: David Widmann <[email protected]>
This prevents constant propagation of the length of a static array. Alternative to #707 Co-authored-by: David Widmann <[email protected]>
The non-concretely typed vector with chunks prevents constant propagation of the length of a static array. Alternative to #707
This code was used to workaround JuliaLang/julia#29887 but with modern Julia it just inhibits optimizations.