WIP: implement #4916, keyword args as dictionary

[JeffBezanson]

This seems like a good change to me, and while some details are up in the air the important thing is the API change (namely, rest keywords being dict-like and immutable). Haven't run all tests yet, but this went pretty smoothly so far.

I think immutability really makes sense here since there tend to be few keyword args, and function arguments tend to be used in an immutable way. Positional varargs are of course already immutable. This already eliminates some allocations (unnecessary empty Vector{Any}s), and at least for small argument counts I expect this to speed up the hard-to-optimize uses of keyword args.

I'm not sure what type to use here but ImmutableDict{Symbol,Any} seems to work well for now.

Previously, rest keywords were sloppy about duplicates:

julia> ks(;x...) = x;

julia> ks(;[:a=>1]..., [:a=>2]...)
2-element Array{Any,1}:
 (:a, 1)
 (:a, 2)

With this change, we use the constructed dict to de-duplicate the keyword names (keeping only the last one):

julia> ks(;[:a=>1]..., [:a=>2]...)
Base.ImmutableDict{Symbol,Any} with 1 entry:
  :a => 2

However, this is potentially costly since ImmutableDict lookup is O(n).

At the same time, rest keywords need to preserve the order they were passed in, and ImmutableDict is perfect for that since it happens to be ordered. I know I've written code that assumes keyword arg order is preserved, but I guess we can consider dropping that guarantee.

[JeffBezanson]

Highly-biased benchmark:

Before:

f(;kw...) = g(;kw...)
g(;kw...) = kw
c = g(a=1,b=2)

julia> @time for i = 1:10000; f(a=1,b=2); end
  0.034662 seconds (140.00 k allocations: 7.019 MiB)

julia> @time for i = 1:10000; f(;c...); end
  0.056522 seconds (180.00 k allocations: 8.240 MiB)

after:

julia> @time for i = 1:10000; f(a=1,b=2); end
  0.003358 seconds (30.00 k allocations: 1.678 MiB)

julia> @time for i = 1:10000; f(;c...); end
  0.000678 seconds

[alyst]

👍 Does it also improve type inference (e.g. #21811, #21794)?

[JeffBezanson]

No this has no effect on type inference.

[quinnj]

I have some unpublished (and unpolished) code that actually exploited the fact that you could pass in duplicate keyword arguments; just to confirm, this would not allow duplicate keyword args, right? Not saying we shouldn't do this, but just confirming.

[JeffBezanson]

You're still allowed to pass duplicate keyword arguments, but they get de-duplicated. However the implementation of ImmutableDict allows multiple mappings for the same key, so with the current state of this PR it's possible to just remove the de-duplication if we want. However that doesn't fit so well with the args[name] dictionary interface, and might be harder to support with other implementations of the keyword args container.

[andyferris]

Cool!

I've been playing with the idea of using a strongly-typed immutable dictionary for the keyword arguments, with a proof-of-principle demonstration using macros at the definition and call sites (this is still early WIP but I'm confident it would work) with the idea that lowering could implement the macro behavior automatically...

So I think we could have all the things (dictionary structure for the varargs (rest?) keywords, plus type stability) without too much fuss - the main difference to this being using a strongly-typed dict instead of ImmutableDict.

[JeffBezanson]

Yes, I think it's likely we will eventually use whatever form of built-in NamedTuple type we end up with for this.

Here it gets interesting, since from my experience with NamedTuples (and also true of namedtuples in python), the elements should just be the values, and not key=>value pairs, since the names are part of the structure (or type), and not part of the data. That raises the question of what the interface for keyword argument providers should be. Currently we expect them to be (1) iterable, and (2) generate elements that can be iterated to yield 2 values (a name Symbol and a value). That works, but it seems ad-hoc and inelegant to me, and doesn't fit well with more strongly-typed containers where the type of the value can depend on the name. Anybody have any ideas?

[bramtayl]

Well, there could be a way to iterate over objects by field name? Maybe

for (field, value) in overfields(t)

[ajkeller34]

I'm not sure if this is a good idea, but since this PR suggests using an ImmutableDict as a keyword argument provider, here are the bones of a type-stable ImmutableDict-like object, along with a simple example showing that it remains type-stable when a duplicate key is entered with an associated value of a different type than before. I something like this could work if passed to a kwsorter (provided iteration, etc. were implemented).

https://gist.github.com/ajkeller34/9987cfcc53660cfbb5349965fc6a2a52

Then again, the type signatures become pretty ungainly, so the elegance criterion may not be satisfied.

[JeffBezanson]

Thanks Andrew, that's pretty clever. I think ultimately we will have to use a standard NamedTuple type (ala #16580) for this.

[quinnj]

So is the idea to have a fully baked NamedTuple first and then implement this using that? Or make this change 1st w/ NamedTuple at some future date?