Luke Kanies <[EMAIL PROTECTED]> writes:
> On Jul 18, 2008, at 12:28 AM, Daniel Pittman wrote:
>
>> * Implement Puppet::Parser::Scope#each_var to yield all variables
>> and values within
>> the current scope, and optionally into the parent.
>> * Use that to set instance member variables into
>> Puppet::Parser::Templatewrapper
>> * Rename the @scope member of the template wrapper, to avoid
>> clashing with a scope
>> variable exposed within puppet.
>
>
> Any idea of the performance implications of this?
> Seems like it would be more expensive, but I don't know how much more.
>
> As long as it's not 10x more expensive or anything, I'm not against
> this basic patch, although I'd like to see tests.
Well, some ad-hoc testing tells me that we are looking at ~ 5µs per
variable and 5µs per level of nesting on a 2.6GHz CPU, which should
vanish into the noise floor in most cases, I think.
However, testing is sensible: I can't see any existing tests for
performance, which I imagine is what you are primarily looking for.
Are there examples that I have missed or, if not, can you give any
direction on how best to write them? I am concerned that my relative
lack of experience with Ruby will lead to bad design choices there.
> I figure we can actually deprecate the current style as part of 0.25
> (with warnings, for a while).
Not done yet: I don't know the best approach to implementing that within
Puppet, and would value your advice.
[...]
> Plese try to match styles with the rest of the system. We always use
> four spaces for indentation, and we put parans around method
> definitions.
Sorry, that is quite a correct complaint. Fixed.
> Also, I think it makes more sense to just support a to_hash method for
> scopes, again with the optional 'recurse' option.
Return a hash of variable => value, then iterate that externally? Hrm.
That seems reasonable, and certainly improves performance over nested
yields.
Exposing a block like this seemed more in keeping with common Ruby
style, but I have no strong feelings. Done.
[...]
> You should be able to modify (or make, if necessary) an accessor that
> returns the value of @__scope__, to avoid always having to use the
> underscores.
That is cleaner, even for a deprecated feature. Done. I will send the
updated patch for consideration in a moment.
Regards,
Daniel
--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups
"Puppet Developers" group.
To post to this group, send email to [email protected]
To unsubscribe from this group, send email to [EMAIL PROTECTED]
For more options, visit this group at
http://groups.google.com/group/puppet-dev?hl=en
-~----------~----~----~----~------~----~------~--~---
