Jira (PUP-10113) Functions are slower if called with chained syntax than with prefix syntax

48 views
Skip to first unread message

Dimitri Tischenko (JIRA)

unread,
Oct 28, 2019, 10:46:02 AM10/28/19
to puppe...@googlegroups.com
Dimitri Tischenko created an issue
 
Puppet / Bug PUP-10113
Functions are slower if called with chained syntax than with prefix syntax
Issue Type: Bug Bug
Affects Versions: PUP 5.5.16
Assignee: Unassigned
Attachments: test.log, test.pp
Created: 2019/10/28 7:45 AM
Environment:

test.pp

test.log

Priority: Normal Normal
Reporter: Dimitri Tischenko

Puppet Version: 5.5.17
Puppet Server Version: 2018.1
OS Name/Version: Centos 7

 

Desired Behavior:

As documented [here|https://puppet.com/docs/puppet/5.5/lang_functions.html#chained-function-calls], chained function call syntax should result in same behaviour as prefixed syntax.

Actual Behavior:

Using chained syntax results in slower performance of the function call. The performance depends on the size of the object "before the dot".

Examples:
Run `puppet apply test.pp --debug --profile`

Please see attached:

  • test.pp
  • test.log
Add Comment Add Comment
 
This message was sent by Atlassian JIRA (v7.7.1#77002-sha1:e75ca93)
Atlassian logo

Dimitri Tischenko (JIRA)

unread,
Oct 28, 2019, 10:47:03 AM10/28/19
to puppe...@googlegroups.com

Dimitri Tischenko (JIRA)

unread,
Oct 28, 2019, 10:48:02 AM10/28/19
to puppe...@googlegroups.com
Dimitri Tischenko updated an issue
 
Change By: Dimitri Tischenko
*Puppet Version: 5.5.17*
*Puppet Server Version: 2018.1*
*OS Name/Version: Centos 7*

 

*Desired Behavior:*

As documented
in [ here|[ https://puppet.com/docs/puppet/5.5/lang_functions.html#chained-function-calls] ] , chained function call syntax should result in same behaviour as prefixed syntax.

*Actual Behavior:*


Using chained syntax results in slower performance of the function call. The performance depends on the size of the object "before the dot".

{{ Examples: }}
{{ Run ` puppet apply test.pp --debug --profile ` }}


Please see attached:
* test.pp
* test.log

Dimitri Tischenko (JIRA)

unread,
Oct 28, 2019, 10:49:02 AM10/28/19
to puppe...@googlegroups.com
Dimitri Tischenko updated an issue
*Puppet Version: 5.5.17*
*Puppet Server Version: 2018.1*
*OS Name/Version: Centos 7*

 

*Desired Behavior:*

As documented in [https://puppet.com/docs/puppet/5.5/lang_functions.html#chained-function-calls], chained function call syntax should result in same behaviour as prefixed syntax.


*Actual Behavior:*

Using chained syntax results in slower performance of the function call. The performance depends on the size of the object "before the dot".

{{Examples:}}

{{ Run puppet apply test.pp --debug --profile}}

Please see attached:
* test.pp
* test.log

Henrik Lindberg (JIRA)

unread,
Oct 28, 2019, 10:52:03 AM10/28/19
to puppe...@googlegroups.com
Henrik Lindberg commented on Bug PUP-10113
 
Re: Functions are slower if called with chained syntax than with prefix syntax

The peformance issue is this bit in the evaluator:

  def eval_CallMethodExpression(o, scope)
    unless o.functor_expr.is_a? Model::NamedAccessExpression
      fail(Issues::ILLEGAL_EXPRESSION, o.functor_expr, {:feature=>'function accessor', :container => o})
    end
    receiver = unfold([], [o.functor_expr.left_expr], scope)
    name = o.functor_expr.right_expr
    unless name.is_a? Model::QualifiedName
      fail(Issues::ILLEGAL_EXPRESSION, o.functor_expr, {:feature=>'function name', :container => o})
    end
    name = name.value # the string function name
    obj = receiver[0]
    receiver_type = Types::TypeCalculator.infer(obj)
    if receiver_type.is_a?(Types::TypeWithMembers)
      member = receiver_type[name]
      unless member.nil?
        args = unfold([], o.arguments || [], scope)
        return o.lambda.nil? ? member.invoke(obj, scope, args) : member.invoke(obj, scope, args, &proc_from_lambda(o.lambda, scope))
      end
    end
    call_function_with_block(name, unfold(receiver, o.arguments || [], scope), o, scope)
  end

Line 13 in this snippet does a full inference to determine if the object has member attributes. For a very large facts hash, that is a very expensive operation.
We need to quickly filter out values that we know cannot possibly have methods, and possibly also find a much faster way to determine if object has members that can be called.

A very simple hack is to add:

unless obj.is_a?(Hash) || obj.is_a?(Array)

but we probably want to speed this up for all Scalar. Ping Thomas Hallgren - any thoughts on what would be the fastest here?

This also brings up a possibility to cache type inference for larger data types...

Thomas Hallgren (JIRA)

unread,
Oct 28, 2019, 11:29:03 AM10/28/19
to puppe...@googlegroups.com

It's likely that we have the same performance issue in pops/evaluator/access_operator.rb (method accessObject line 31).

I think a specialized infer_callable (or similar name) method that would return nil in case the given value don't have that such a type could make a lot of difference. AFAICT, only PObjectType, PObjectTypeExtension and PURIType implements TypeWithMembers.

The current @api private method TypeCalculator.infer_Object could serve as model for this new method.

Jorie Tappa (JIRA)

unread,
Oct 28, 2019, 12:48:02 PM10/28/19
to puppe...@googlegroups.com

Jorie Tappa (JIRA)

unread,
Oct 28, 2019, 12:48:03 PM10/28/19
to puppe...@googlegroups.com

Dimitri Tischenko (JIRA)

unread,
Oct 29, 2019, 2:45:04 PM10/29/19
to puppe...@googlegroups.com

Dimitri Tischenko (JIRA)

unread,
Oct 29, 2019, 2:46:03 PM10/29/19
to puppe...@googlegroups.com

Henrik Lindberg (JIRA)

unread,
Oct 30, 2019, 5:30:04 AM10/30/19
to puppe...@googlegroups.com

Henrik Lindberg (JIRA)

unread,
Oct 30, 2019, 5:48:03 AM10/30/19
to puppe...@googlegroups.com
Henrik Lindberg assigned an issue to Unassigned

Henrik Lindberg (JIRA)

unread,
Oct 30, 2019, 5:52:03 AM10/30/19
to puppe...@googlegroups.com
Henrik Lindberg commented on Bug PUP-10113
 
Re: Functions are slower if called with chained syntax than with prefix syntax

Josh Cooper I made a PR for this as I had figured out how to repair this when investigating the cause. I made the PR against master, but I think it is trivially backported to 5 as well. Think this should go in soonest as it makes a dramatic difference.

Josh Cooper (JIRA)

unread,
Oct 30, 2019, 2:15:04 PM10/30/19
to puppe...@googlegroups.com
Josh Cooper commented on Bug PUP-10113

Henrik Lindberg your comment about large fact hash makes me think this is the cause of PUP-9577?

Henrik Lindberg (JIRA)

unread,
Oct 30, 2019, 2:33:04 PM10/30/19
to puppe...@googlegroups.com

Josh Cooper Could be - it is one likely explanation. It will not only take cycles to produce the inferred type (that will be avoided when the PR is merged), it also builds a resulting inferred type with full inference, so it contains as many objects as there is content in the complete hash, then that type information is simply thrown away - result is a lot of garbage.

The PR makes both chained calls and access with [ ] faster as it also needs to check if the LHS supports calling a method for that operator. I think this is quite common - for example using $facts[somefact].

Thomas Hallgren (JIRA)

unread,
Oct 30, 2019, 3:30:04 PM10/30/19
to puppe...@googlegroups.com

Just a side note regarding type inference. Perhaps it would be worth investigating a concept I called "exact types" in dgo. In essence, an exact type is a type that is backed by the value that it represents. Complex types like map, array, etc. always return that kind of type which means that very little copying is made in order to determine the type of a value. A full test if such a type is assignable to another type is the same thing as asking if the value that it represents is an instance of the that type.

I don't think it's trivial to do this in Puppet but it's probably doable without too much of an effort.

Charlie Sharpsteen (JIRA)

unread,
Jan 8, 2020, 7:31:03 PM1/8/20
to puppe...@googlegroups.com

Proposed PR:

https://github.com/puppetlabs/puppet/pull/7797

The patchset appears to have yielded a nice 10 – 20% performance improvement when applied to a customer site that uses chained functions heavily.

Josh Cooper (JIRA)

unread,
Jan 16, 2020, 1:58:05 PM1/16/20
to puppe...@googlegroups.com

Josh Cooper (JIRA)

unread,
Jan 16, 2020, 1:58:05 PM1/16/20
to puppe...@googlegroups.com

Josh Cooper (JIRA)

unread,
Jan 16, 2020, 1:58:06 PM1/16/20
to puppe...@googlegroups.com

Josh Cooper (JIRA)

unread,
Jan 16, 2020, 7:44:03 PM1/16/20
to puppe...@googlegroups.com
Josh Cooper updated an issue
Change By: Josh Cooper
Release Notes Summary: The puppet language supports calling functions using prefix or chained syntax,  for example, "each($var)" and "$var.each", respectively. Using the latter syntax is now much faster, especially when $var is a large hash, such as $facts from the node.
Release Notes: Bug Fix

Josh Cooper (JIRA)

unread,
Jan 16, 2020, 7:44:04 PM1/16/20
to puppe...@googlegroups.com
Josh Cooper updated an issue
Change By: Josh Cooper
Fix Version/s: PUP 6.4.6
Fix Version/s: PUP 5.5.19
Fix Version/s: PUP 6.13.0

Josh Cooper (JIRA)

unread,
Jan 22, 2020, 7:17:06 PM1/22/20
to puppe...@googlegroups.com

Melissa Stone (JIRA)

unread,
Jan 28, 2020, 6:06:04 PM1/28/20
to puppe...@googlegroups.com

This has passed ci as a part of puppet-agent 5.5.18.44.g1c1c714b

Kate Medred (JIRA)

unread,
Feb 18, 2020, 12:12:14 PM2/18/20
to puppe...@googlegroups.com
Reply all
Reply to author
Forward
0 new messages