use unsafe_views and remove branch for arrays

[KristofferC]

An alternative for solving #83

julia> println("distances.jl colwise")
distances.jl colwise

julia> show(STDOUT, MIME"text/plain"(), bench_colw)
BenchmarkTools.Trial:
  memory estimate:  781.33 KiB
  allocs estimate:  2
  --------------
  minimum time:     1.082 ms (0.00% GC)
  median time:      1.256 ms (0.00% GC)
  mean time:        1.364 ms (2.21% GC)
  maximum time:     3.484 ms (24.77% GC)
  --------------
  samples:          3644
  evals/sample:     1
julia> println("\nnaive colwise")

naive colwise

julia> show(STDOUT, MIME"text/plain"(), bench_colw2)
BenchmarkTools.Trial:
  memory estimate:  781.33 KiB
  allocs estimate:  2
  --------------
  minimum time:     1.072 ms (0.00% GC)
  median time:      1.275 ms (0.00% GC)
  mean time:        1.354 ms (2.25% GC)
  maximum time:     3.658 ms (32.67% GC)
  --------------
  samples:          3670
  evals/sample:     1

cc @chethega

[KristofferC]

Note this @inline. It apparently does quite a lot for the performance with small arrays and it isn't that much code to inline.

[chethega]

I think the unsafe_views beats the split-view API in terms of code-readablity and should be equivalent in terms of speed.

I will try to port over the better memory access / multithreaded pairwise!, as well as the modified boundscheck-propagation from my PR; then my the split-views can be dropped.

Question: I think distances.jl is unhappy with one-dimensional indices not running from 1:length(V)? As in e.g. offset-arrays?

[KristofferC]

Yeah, this package was created a long time ago and the core has been largely unchanged. It is likely it doesn't work well with non one based arrays. We should fix that and test it.

[chethega]

You can find the more memory-friendly iteration order on https://gist.github.com/chethega/00241c6312d3fe28afc5532708a69b3d.

After some testing, I could not see any significant performance gains; therefore, it probably does not belong in Distances.jl.

[KristofferC]

On JuliaLang/julia#24113, with normal views, I see no allocations as long as I inline evaluate. Performance is the same as the hand rolled one as well.

We could put this in for 0.6 and remove it when tagging a 0.7 release.

[chethega]

Sure. Then, instead of exporting and cementing the fastview, export and document now something like

@inline @Base.propagate_inbounds evaluate(d, A, jA, B, jB)= evaluate(d, fastview(A,:,jA), fastview(A,:,jB))

and, after removal of fastview, replace it by

@inline @Base.propagate_inbounds evaluate(d, A, jA, B, jB)= evaluate(d, view(A,:,jA),view(A,:,jB)),

and possibly deprecate this a couple versions later? Then one should maybe add boundscheck to fastview, and be happy.