commit 972ade8b70fa55b53b56043ac1bbfac5f3bbc8fd
author adonovan <adonovan@google.com> Wed Dec 18 13:58:14 2019 -0800
committer Copybara-Service <copybara-worker@google.com> Wed Dec 18 13:59:13 2019 -0800
tree 1bf0f5775d342f434186d582e8fa9468fa9e1d61
parent 61216fd405e9b80ce89ffafbb6518e859679f4ab

bazel syntax: fast calling convention

This CL changes the calling convention so that the caller provides
the callee with two arrays, one of positional and one of named arguments,
the latter represented as alternating String/Object values.
The array of named arguments seen by the callee may contain duplicates.
This reduces allocation, especially of hash tables, as the callee
must already check for duplicates implied by the combination of positional
and named.

BaseFunction historically adapted an "outer" call protocol to an "inner"
one, interpreting the positional and named arguments with respect to a
FunctionSignature. The change moves the task of adaptation into the base
interface, StarlarkCallable, so that subclasses may implement either
the outer protocol (fastcall) or the inner one (call)---or neither for
and "unimplemented" test or fake function. The inner protocol now simply
collects all the positional and named arguments and presents them as a
tuple and a dict, as if by the signature def f(*args, **kwargs).

BaseFunction's role is reduced to implementing methods like toString
and repr based on the signature and default values, and exposing this
information to the documentation tools. (The default behaviors of
these methods could be improved.) The signature and default values
are not implicitly used for any kind of validation, though subclasses
may use them explicitly for that purpose.

BaseFunction.processArguments has moved to Starlark.checkSignature,
making Signature-based argument processing available to classes unrelated
to BaseFunction. This function could be much improved in clarity and UI,
but in this CL I have limited edits to performance concerns:
- eliminating the 'bothPosKey' pass and allocation (which means we don't
  distinguish the causes of repeated argument).
- case 2b deleted (it's no longer a faster version of 2c)
- avoid duplicate hashing when inserting to kwargs.
I plan to improve the code and the UI (error messages) in a follow-up.

Also
- turn BaseFunction.signature field into an abstract method;
  for most subclasses it is a constant.
- PackageFactory.newPackageFunction uses the inner protocol
  and does its own parameter validation. Simpler than
  forcing it to use FunctionSignature.
- avoid dubious list mutation in BuiltinCallable

The performance impacts have been hard to measure, with benchmarks
showing high variance. A few ad-hoc tests of particularly expensive
single BUILD files have shown consistent reductions in real time of
around 15%.

PiperOrigin-RevId: 286259755