more about type annotation #23180
more about type annotation #23180
Conversation
Type annotation not recommended for types constructed at run-time
doc/src/manual/performance-tips.md
Outdated
| end | ||
| ``` | ||
|
|
||
| the annotation of `c` harms performance. To write performant code involving types constructed at |
There was a problem hiding this comment.
Does it actually harm performance? Or just not help it?
There was a problem hiding this comment.
Actually harms... I tried it out with benchmarktools in 0.6:
function test4(a, prec)
ctype = prec == 32? Float32 : Float64
b = Complex{ctype}(a)
c = (b + 1.0f0)::Complex{ctype}
norm(c)
end
function test5(a, prec)
ctype = prec == 32? Float32 : Float64
b = Complex{ctype}(a)
c = (b + 1.0f0)
norm(c)
endjulia> @benchmark test_convert.test4(5.0f0, 32)
BenchmarkTools.Trial:
memory estimate: 64 bytes
allocs estimate: 4
--------------
minimum time: 391.460 ns (0.00% GC)
median time: 398.505 ns (0.00% GC)
mean time: 436.490 ns (0.32% GC)
maximum time: 3.565 μs (81.16% GC)
--------------
samples: 10000
evals/sample: 202
julia> @benchmark test_convert.test5(5.0f0, 32)
BenchmarkTools.Trial:
memory estimate: 64 bytes
allocs estimate: 4
--------------
minimum time: 248.485 ns (0.00% GC)
median time: 255.462 ns (0.00% GC)
mean time: 267.307 ns (0.50% GC)
maximum time: 1.427 μs (78.86% GC)
--------------
samples: 10000
evals/sample: 530
There was a problem hiding this comment.
Huh, that's interesting. I guess the compiler can't prove that the error can't happen and has to generate error checking/handling code in addition to the already-slow dynamic dispatch.
doc/src/manual/performance-tips.md
Outdated
|
|
||
| ```julia | ||
| function nr(a, prec) | ||
| ctype = prec == 32? Float32 : Float64 |
There was a problem hiding this comment.
note that ternaries without spaces on both sides of ? are deprecated on master, so we should avoid the deprecated syntax in doc examples
As per @tkelman 's suggestion, I fixed the deprecated syntax in the snippet. I also reworded the advice at the end of the new text to clarify that parameterization is just one possible way to get performant code, but the key point is that the constructed type should be in the type signature of the arguments of the kernel.
doc/src/manual/performance-tips.md
Outdated
|
|
||
| does not hinder performance (but does not help either) since the compiler can determine the type of `c` | ||
| at the time `k` is compiled. | ||
|
|
There was a problem hiding this comment.
This addition is beautifully written :).
Changed double-back-quotes to single-back-quotes and removed needless spaces as per suggestions from T.K.
doc/src/manual/performance-tips.md
Outdated
| @@ -485,6 +485,34 @@ annotation in this context in order to achieve type stability. This is because t | |||
| cannot deduce the type of the return value of a function, even `convert`, unless the types of | |||
| all the function's arguments are known. | |||
|
|
|||
| Type annotation will not enhance (and can actually hinder) performance if the type is constructed | |||
| at run-time. This is because the compiler cannot use the annotation to specialize the subsequent | |||
There was a problem hiding this comment.
run-time or runtime?
Edit: Seems to be 50/50 across the repo so :)
kshyatt
added
docs
As per suggestion from F.E., I removed double-spaces at the end of sentences (after periods). I did not change "run-time" to "runtime" or "run time" because it appears that "run-time" is used throughout this section of the manual.
|
@fredrikekre, please go ahead and merge this as soon as you're happy with your edits to it. @StephenVavasis, thanks for the nice addition! |
|
Would be nice if at least some workers ran for more than 13 minutes... Seems all of them got some spurious failure. Restarting Travis, hopefully one worker green, then merging. |
|
The failures are actually related to this change. A bunch of references in this file is messed up somehow: !! Reference for 'Pre-allocating-outputs' could not be found. [src/manual/performance-tips.md]
!! Reference for 'kernal-functions' could not be found. [src/manual/performance-tips.md]
!! Reference for 'Pre-allocating-outputs' could not be found. [src/manual/functions.md]
!! Reference for 'man-performance-annotations' could not be found. [src/stdlib/math.md]But references to |
Apparently the inclusion of the section ref in my new text is breaking the text correctness-checking system. Since I don't have the time or knowledge to debug the correctness-checker, I have removed the ref in the hopes that this text will now succeed in the checker.
|
I still get !! Reference for 'Pre-allocating-outputs' could not be found. [src/manual/performance-tips.md]
!! Reference for 'Pre-allocating-outputs' could not be found. [src/manual/functions.md]
!! Reference for 'man-performance-annotations' could not be found. [src/stdlib/math.md]but I have no idea why... |
|
Okay, there was an extra space causing all the troubles... Now it works locally, lets hope for CI to be green then lets merge! |
|
#23371 on one Travis worker |
Type annotation not recommended for types constructed at run-time