Improve performance of edge iters

[simonschoelly]

This PR improves the performance of the edge iterators. It also fixes a bug, where the edge iterator would not work, when nv(g) == typemax(eltype(g)).

The previous implementation was for SimpleAbstractGraph, so I kept that by using SimpleTraits for distinguishing between directed and undirected graphs, but all other implementations in the SimpleGraphs submodule seem to be only implemented for SimpleGraph and SimpleDiGraph, so we might also do that.

Some benchmarks for collect, it might still be useful to implement collect directly though (see also #1201 )

# sparse undirected

julia> g = dorogovtsev_mendes(10000, seed=1);

## old
julia> @btime collect(edges(g));
  342.597 μs (3 allocations: 312.59 KiB)

## new
julia> @btime collect(edges(g))
  154.607 μs (3 allocations: 312.59 KiB)


# dense undirected

julia> g = erdos_renyi(Int16(5000), 0.5, seed=1);

## old
julia> @btime collect(edges(g));
  86.561 ms (3 allocations: 23.83 MiB)

julia> @btime collect(edges(g))
  13.419 ms (3 allocations: 23.83 MiB)


# dense directed

julia> gd = SimpleDiGraph{Int16}(erdos_renyi(3500, 0.5, is_directed=true, seed=1));

## old
julia> @btime collect(edges(gd));
  26.299 ms (3 allocations: 23.36 MiB)

## new
julia> @btime collect(edges(gd))
  11.228 ms (3 allocations: 23.36 MiB)

[codecov]

Codecov Report

Merging #1202 into master will decrease coverage by <.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #1202      +/-   ##
==========================================
- Coverage   99.62%   99.62%   -0.01%     
==========================================
  Files          94       94              
  Lines        4317     4311       -6     
==========================================
- Hits         4301     4295       -6     
  Misses         16       16

[abhinavmehndiratta]

Why use SimpleTraits here ? Not every subtype of AbstractSimpleGraph is required to have a fadj function, so I guess we should either use neighbors or remove @traitfn and use SimpleGraph or SimpleDiGraph instead.

[simonschoelly]

The problem with with neighbors is, that it does not propagate @inbounds. Maybe it should, but as long as it does not do that, we would have an additional bound check in every iteration.

[sbromberger]

@simonschoelly is this ready to go, or is there still an issue with overloading an unexported function from Base?

[simonschoelly]

I think you are confusing this PR with #1137 - we are not overloading any unexported functions here.
@abhinavmehndiratta had some objections, so I don't know if I still should do some changes here - otherwise we might merge this.

[sbromberger]

I think you are confusing this PR with #1137

I am. Thanks for setting me straight.

I'm generally ok with adding SimpleTraits here since it's a good use of the function, but I note that you could probably pull the inner loops out into their own functions and use is_directed() to call whichever one works. This might have a performance impact but I can't imagine it'd be too big.

[simonschoelly]

So, should I write a version using is_directed or will we merge it like this?

[sbromberger]

I think we can merge this as it stands.