Add keyword argument and functors support to invoke (invoke improvement No. 4)

[yuyichao]

I don't expect this can be directly accept but I would like to post it in order to get some feedback.

A few things to note about this PR.

1. I'm not sure if this is the right place to add/export these. invoke is not used a lot in Base and they can directly use Core._invoke if necessary so I think it shouldn't be hard to make this dicision later.
2. This is a port of an experiment in one of my other packages (where I started with only simple cases but later realized it was not so hard to support a wider range either) so it brought in some helper functions as well. Please comment if any of those should be changed / merged with available functions. (Related "Regression" in typeinf when concatenate two tuple types #10880)
3. There's currently no test mainly because it is blocked by Wrong result for kwsorter-for-call-like function #11149 but should be fixed by Fix invoke for vararg methods. #11151 .
4. There's a type inference regression caused by Constant folding/propagation of functions #11159. However, IIRC the performance loss is not so big (mainly because invoke is pretty expensive right now anyway....)
5. There's also a hack to get faster keyword argument forwarding (by directly defining kwsorter) which is enabled by default now. Is this acceptable? IMHO, this should probably be treated as an internal API and therefore is probably not a great idea for a package but maybe ok for Base.

I have some mini benchmark for 4 and 5 for my old code but I would need to clean it up later.

[yuyichao]

P.S. I can imagine that the regression in type inference might cause failing tests in some corner cases. Those can be easily fixed by replacing invoke with _invoke (although ideally #11159 should be fixed) so I just left it as is for now and see how bad it is on CI.

P.P.S. does julia do any constant folding at AST (typeinf) level at all?

[carnaval]

Don't hesitate to ping @JeffBezanson for review. I think usually this kind of stuff is done more by adding specialized codegen for the builtin in codegen.cpp. It may be easier to generate the code you want directly than try to coerce julia into doing it. In particular I'm not sure about the staged function stuff.

Regarding the const. prop., we only do it for types for now. I'm currently working on something that should make it easy to have const. prop. at the julia level (and hopefully more). It's not usable (read : cannot modify the code!) now but I hope to have something somewhat soon. If you want to discuss it before that it may be easier to just send me an email.

[yuyichao]

@carnaval IMHO this is the "minimum" way to add this support since otherwise there's probably much more places to change and would introduce a even higher barrier for performance optimization.

Don't hesitate to ping @JeffBezanson for review.

Noted =) . Just wondering how often can I do that to annoy a superhero.

[yuyichao]

@carnaval
Performance wise I had an earlier version of this implementation when I had #9642 and AFAICT apart from the constant propagation issue, the inlining pass can optimize calls to invoke pretty efficiently. If the support is added to Core.invoke instead, it will probably be harder to model and can introduce more bugs.

Also. the builtin functions functions cannot support keyword arguments (which was the original goal of the PR, the functor support is actually just a after thought). Changing this might have a much bigger impact than the current PR as well.

[carnaval]

For the callable type part :

It may be a little more painful to do the change in the codegen (and in the builtin) but I don't see why it would be a barrier to perf. optimization. In fact it would avoid specializing the invoke method for every callable argument and you can take advantage of the infrastructure which already does the callable/function distinction with a fast runtime branch (doing something like what is done for jl_apply_generic https://github.com/JuliaLang/julia/blob/master/src/codegen.cpp#L1912)

For the keyword args I'm not sure. If we wanted to keep invoke a builtin (and not a generic func) we would have to add a special case to kwcall. Is that all ?

[yuyichao]

In fact it would avoid specializing the invoke method for every callable argument

This actually brings up one of my confusion. Is this an issue if the function is always inlined?

[carnaval]

Yep I'd agree that keeping kw args only for generic functions seems easier for now. I think however that the call function stuff would be better reusing what we have now in codegen. Let's see if someone has something to say about this.

This actually brings up one of my confusion. Is this an issue if the function is always inlined?

Compilation times are the primary problem I'd say. Invoke is essentially unused so probably not an issue in practice.

[yuyichao]

Just adding cross references:
This PR closes #7045 (and I just noticed that I posted an earlier/simplified version of this PR there before...)
Somewhat related to #9498 (or at least this is how I discovered that) since the current implementation has the same "limitation".

[yuyichao]

Use tuple_type_cons

[yuyichao]

Rebased due to conflict with #11274

[quinnj]

How does this fare post jb/functions?

[yuyichao]

FWIW, invoke still doesn't support kw argument now, although this also needs to be heavily rebased...