Anti-aliasing false positive with same array

roflmaostc commented 2 years ago

Hi!

I encountered an anti-aliasing bug:

See, on

julia> versioninfo()
Julia Version 1.7.0
Commit 3bf9d17731 (2021-11-30 12:12 UTC)
Platform Info:
  OS: Linux (x86_64-pc-linux-gnu)
  CPU: Intel(R) Core(TM) i7-7600U CPU @ 2.80GHz
  WORD_SIZE: 64
  LIBM: libopenlibm
  LLVM: libLLVM-12.0.1 (ORCJIT, skylake)
Environment:
  JULIA_NUM_THREADS = 4

, this example:

julia> A = 200*rand(360,180,1200);

julia> capnan0!(A,v) = A[A .> v] .= NaN
capnan0! (generic function with 1 method)

julia> capnan0!(copy(A), 100); # happy compilation

julia> @time capnan0!(A, 100); @time capnan0!(A, 100); @time capnan0!(A, 100);
  0.331328 seconds (9 allocations: 305.919 MiB, 18.35% gc time)
  0.071091 seconds (8 allocations: 9.274 MiB)
  0.071649 seconds (8 allocations: 9.274 MiB)

julia> A = 200*rand(360,180,1200);

julia> @time capnan0!(A, 100); @time capnan0!(A, 100); @time capnan0!(A, 100);
  0.262531 seconds (9 allocations: 305.878 MiB)
  0.070990 seconds (8 allocations: 9.274 MiB)
  0.070394 seconds (8 allocations: 9.274 MiB)

julia> @time capnan0!(A, 100); @time capnan0!(A, 100); @time capnan0!(A, 100);
  0.070696 seconds (8 allocations: 9.274 MiB)
  0.070205 seconds (8 allocations: 9.274 MiB)
  0.070794 seconds (8 allocations: 9.274 MiB)

So whenever (first run) we really do an assignment (with the BitArray) it falsely copies the right hand side (the 305MiB). In further runs (BitArray only 0s), nothing should be done and it also doesn't copy it.

We should be possible to prevent this, right?

Best,

Felix

Further reference:

N5N3 commented 2 years ago

I'm affraid this is not a false positive of Anti-aliasing. Profile shows that the time cost during the first call could be equally (almost) divided into:

p = A .> v
B = view(A, p) (It do 'collect' during ensure_indexable, I think the extra allocation comes from here).
B .= NaN or fill!(B, NaN)

For the second and third call, A has been updated, so A .> v return a all false BitArray, thus the last 2 part go away.

roflmaostc commented 2 years ago

Yeah but why should there be a copy in step 2?

The update is in-place and we are definitely don't assign a copy but instead the original one. That seems to wok well.

KristofferC commented 2 years ago

Might be similar to https://github.com/JuliaLang/julia/issues/30041.

N5N3 commented 2 years ago

Good to know we aready have a issue to track this. But improvement might be hard, as the current api returns a Subarray. If we can't prove there's no further access on the Subarray, the collect of LogicalIndex seems unavoidable. (Edit: But looks like we can use findall to make it faster, see https://github.com/JuliaLang/julia/issues/30041#issuecomment-996708441)

JuliaLang / julia

Anti-aliasing false positive with same array #43442