Jacob and I were going through patches submitted to the mailing list. It looks like there were some changes that you were discussing with Markus. Have you had a chance to make these changes since we weren't able to find a revised patch in the mailing list, or were we misreading the dialog between you and Markus?
Josh On Feb 27, 2:23 pm, Brice Figureau <[email protected]> wrote: > On 27/02/11 20:44, Markus Roberts wrote: > > > > > > > > > > > B -- > > >> + # Triggers aninstrumentation > > > + # > > + # Call this method around theinstrumentationpoint > > + # Puppet::Util::Instrumentation.instrument(:my_long_computation) do > > + # ... a long computation > > + # end > > + # > > + # This will send an event to all the listeners of > > "my_long_computation". > > + # The same can be achieved faster than using a block by calling > > start and stop. > > + # around the code to instrument. > > + def self.instrument(label, data = {}) > > + self.start(label, data) > > + yield > > + ensure > > + self.stop(label) > > + end > > + > > + def self.start(label, data = {}) > > + data[:started] = Time.now > > + Thread.current[label] = data > > + publish(label, :start, data) > > + end > > + > > + def self.stop(label) > > + if data = Thread.current[label] > > + Thread.current[label] = nil > > + data[:finished] = Time.now > > + publish(label, :stop, data) > > + else > > + raise "No matching start event for #{label}" > > + end > > + end > > > It appears on first reading that this may fail on if start/stop pairs > > are used in recursive code; specifically, in something like this: > > > def functastic_method > > Puppet::Util::Instrumentation.instrument(:my_long_computation) do > > # ... a long computation that calls functastic_method > > somewhere deep inside > > end > > end > > Yes, I thought about it when I started the code back 3 weeks ago, but > completely forget about it :( > > > that when the next to the last call tries to leave the instrument block > > it will raise "No matching start event..." > > > If we care, this could probably be dealt with by replacing > > Thread.current[label] with something like: > > > Thread.current[:instrumentation] = Hash.new([]) > > > and then doing: > > > Thread.current[:instrumentation][label].push data > > > and the corresponding pop/empty?/etc. > > When I first wrote this, the probe would have been put manually around > body of methods, but now that we meta-program the probes around function > call, we can leverage the new scope we created and, I'm not sure we > really need this trick. > > We can rewrite the probe to do something like: > > call start(label, data) > call original method > call stop(label, data) > > If needed, we can stuff into the data hash an unique id (or return it > from start to send it back to stop) to make sure we can pair the start > and stop event later on. > > > This would also be a good idea to prevent namespace collisions. If > > someone where to choose a label that we're already using internally > > (e.g. :known_resource_types, environment, declarations, etc.) the > > present system would break things in a rather interesting way. > > You're correct, the namespace issue is a problem. > > > > > > > > > > > Finally, I've got an uneasy feeling (but nothing I can stand up and > > explicate) about linking thread data and (global) metaprogramming in > > this way. It's probably fine, but I haven't yet gotten my head around > > all the corners. > > > + def self.publish(label, event, data) > > + listeners_of(label).each do |k,l| > > + l.notify(label, event, data) > > + end > > + end > > > + def self.listeners_of(label) > > + synchronize { > > + @listeners_of[label] ||= @listeners.select do |k,l| > > + l.listen_to?(label) > > + end > > + } > > + end > > > Does this (due to memoization) omit listeners who are added after the > > first publish? > > Each time we enable/disable, we call rehash whose job is to client > listeners_of, so we should be safe here. > -- > Brice Figureau > My Blog:http://www.masterzen.fr/ -- 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.
