speed up methods(f)

[JeffBezanson]

This adds a fast path for getting all (active) methods.

fixes #36860, fixes #38037

[JeffBezanson]

Hmm, do we care about the order? It probably doesn't matter for methods(f) but maybe it does matter for showing the list of closest candidates?

[vtjnash]

Yeah, this is only partially right. I was testing something similar to this myself

[vtjnash]

Suggested change

	elseif (allmethods = methods(f); isempty(allmethods)) && isa(f, DataType) && f.abstract
	elseif isa(f, DataType) && f.abstract && isempty(methods(f))

[vtjnash]

I've got to say though, this conditional makes absolutely no sense to me. I think I get what it's trying to test, but just that it appears completely useless in its current form

Suggested change

	elseif (allmethods = methods(f); isempty(allmethods)) && isa(f, DataType) && f.abstract
	elseif isa(f, Type) && try
	isempty(_methods_by_ftype(Tuple{Type{<:Base.typename(f).wrapper}, Vararg}, -1, typemax(UInt64)));
	catch ex; ex isa ErrorException ? false : rethrow(); end # try/catch because typename may throw for Union

[vtjnash]

Is it still worthwhile to try to fix, or simply delete, this code branch?

[vtjnash]

Suggested change

	elseif isempty(allmethods) && !isa(f, Function) && !isa(f, Type)
	elseif !isa(f, Function) && !isa(f, Type) && isempty(methods(f))

[vtjnash]

For comparison, here's mine (which passes doctest, ambiguous, numbers, reflection, and errorshow):

diff --git a/base/reflection.jl b/base/reflection.jl
index 1d5335ade7..c1b1918547 100644
--- a/base/reflection.jl
+++ b/base/reflection.jl
@@ -857,11 +857,25 @@ function methods(@nospecialize(f), @nospecialize(t),
     world = typemax(UInt)
     # Lack of specialization => a comprehension triggers too many invalidations via _collect, so collect the methods manually
     ms = Method[]
-    for m in _methods(f, t, -1, world)
-        m::Core.MethodMatch
-        (mod === nothing || m.method.module ∈ mod) && push!(ms, m.method)
+    if t == Tuple
+        tt = signature_type(f, t)
+        mt = ccall(:jl_method_table_for, Any, (Any,), tt)::Core.MethodTable
+        visit(mt) do m::Method
+            m.primary_world <= world <= m.deleted_world || return
+            typeintersect(m.sig, tt) === Union{} && return
+            (mod === nothing || m.module ∈ mod) || return
+            push!(ms, m)
+        end
+        lt(a, b) = typeintersect(a.sig, b.sig) === Union{} || Base.morespecific(a.sig, b.sig)
+        sort!(ms, alg=InsertionSort, lt=lt)
+    else
+        for m in _methods(f, t, -1, world)
+            m = m::Core.MethodMatch
+            (mod === nothing || m.method.module ∈ mod) || continue
+            push!(ms, m.method)
+        end
     end
-    MethodList(ms, typeof(f).name.mt)
+    return MethodList(ms, typeof(f).name.mt)
 end
 methods(@nospecialize(f), @nospecialize(t), mod::Module) = methods(f, t, (mod,))
 
@@ -881,7 +895,7 @@ end
 function methods(@nospecialize(f),
                  mod::Union{Module,AbstractArray{Module},Nothing}=nothing)
     # return all matches
-    return methods(f, Tuple{Vararg{Any}}, mod)
+    return methods(f, Tuple, mod)
 end
 
 function visit(f, mt::Core.MethodTable)

[JeffBezanson]

Looks good, let's go with something like that.

[vtjnash]

Sure. However, here's possibly the main counter-example to attempts to improve this:

julia> (::Integer)(x...) = 1

julia> (::Int)(x...) = 2

julia> methods(1, )
# 2 methods:
[1] (::Int64)(x...) in Main at REPL[1]:1
[2] (::Integer)(x...) in Main at REPL[2]:1  # this method should not appear (doesn't appear on master)