Skip to content

Commit

Permalink
(PUP-9561) Optimize parameter validation for blocks
Browse files Browse the repository at this point in the history
Puppet used to infer types for arguments passed to a block. This was slow for
several reasons:

* Type inference is generally slow for large collections (hash/array)
* Iterable functions like `reduce` pass an accumulator to the block for each
  iteration, so we inferred the accumulator's type multiple times.
* When building a collection using `reduce`, the accumulator increases in size
  each time. So constructions like the following infer the type of an
  accumulator of size 0, 1, ..., N-1, which is equivalent to inferring the type
  of an accumulator of size N(N-1)/2:

      Integer[1, N].reduce([]) |$memo, $value| { $memo << $value}

However, block arguments are often defined without a type, e.g. `$memo`, so the
type inference was wasted effort.

This commit switches from `callable?` to `callable_with?`. So rather than
inferring the type of all arguments, and checking if those types are assignable
to the parameter's type, we instead check if each argument is an instance of the
parameter's type. For example, we check if the Ruby Hash {} is an instance of
the `PAnyType`, which is always true[1]. The `callable_with?` method also checks
required and optional parameters and return types, like the `callable?` method
does.

Note the `callable_with?` method was introduced in 2edd30e to address similar
type inference slowness.

[1] https://github.com/puppetlabs/puppet/blob/7.11.0/lib/puppet/pops/types/types.rb#L268
  • Loading branch information
joshcooper authored and luchihoratiu committed Oct 12, 2021
1 parent 7e1d805 commit 64ba729
Showing 1 changed file with 7 additions and 5 deletions.
12 changes: 7 additions & 5 deletions lib/puppet/pops/evaluator/closure.rb
Original file line number Diff line number Diff line change
Expand Up @@ -219,24 +219,25 @@ def enclosing_scope
def call_with_scope(scope, args)
variable_bindings = combine_values_with_parameters(scope, args)

tc = Types::TypeCalculator.singleton
final_args = tc.infer_set(parameters.reduce([]) do |tmp_args, param|
final_args = parameters.reduce([]) do |tmp_args, param|
if param.captures_rest
tmp_args.concat(variable_bindings[param.name])
else
tmp_args << variable_bindings[param.name]
end
end)
end

if type.callable?(final_args)
if type.callable_with?(final_args, block_type)
result = catch(:next) do
@evaluator.evaluate_block_with_bindings(scope, variable_bindings, @model.body)
end
Types::TypeAsserter.assert_instance_of(nil, return_type, result) do
"value returned from #{closure_name}"
end
else
raise ArgumentError, Types::TypeMismatchDescriber.describe_signatures(closure_name, [self], final_args)
tc = Types::TypeCalculator.singleton
args_type = tc.infer_set(final_args)
raise ArgumentError, Types::TypeMismatchDescriber.describe_signatures(closure_name, [self], args_type)
end
end

Expand Down Expand Up @@ -309,6 +310,7 @@ def create_callable_type()
to += param_range[1]
end
param_types = Types::PTupleType.new(types, Types::PIntegerType.new(from, to))
# The block_type for a Closure is always nil for now, see comment in block_name above
Types::PCallableType.new(param_types, nil, return_type)
end

Expand Down

0 comments on commit 64ba729

Please sign in to comment.