> Ok, I've squashed the following changes into the branch.  I believe this
> addresses all the issues Jesse and I found in our code review.
> Regarding the downstream_from_vertex and upstream_from_vertex, I stuck with
> the rewrite Jesse and I suggested because (a) there seemed to be a loose
> majority in favor of it, and (b) in my speed tests it proved to be 1.5 times
> faster.

The loose majority I could shrug off, but empirical data?  A touch, a
touch, I do confess.

But man, they look ugly.

>    # Clear our graph.
>    def clear
> -   �...@in_to.each { |k,v| v.clear }
> -   �...@out_from.each { |k,v| v.clear }

I've been meaning to come back and address these; they were for GC
loop shedding and I recall being fairly confident that they were
needed but I can't reproduce the reasoning at present.  Most likely
they were needed in some earlier (intermediate) version where we still
had the VertexWrapper and the motivating back link has since vanished.
 Another (remote) possibility was worry over some obscure ruby bug,
but I can't think of one that would apply and I'm usually pretty good
at it ("I'll take obscure ruby bugs for 500 please").

It does bring up another question though: what's the present use case
for SimpleGraph#clear, and why do we need to clear the in_to,
out_from, etc. instead of just reinitializing?  Shouldn't the same
reasoning apply there?

-- M

P.S. I also pinged Deepak on this stuff, but haven't heard back from him.
-----------------------------------------------------------
The power of accurate observation is
commonly called cynicism by those
who have not got it.  ~George Bernard Shaw
------------------------------------------------------------

-- 
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.

Reply via email to