+1 with one inlined and one outlined questions > > Quick ruby question: what about using a WeakRef to hold the sync object > and completely get rid of the ref counting? Would that work? >
Interesting idea, but I'm not sure off hand how to make it work. The problem isn't as much the hash values (the Sync objects) as the hash keys (resources, strings, etc.) that can be 1) large and 2) holding references to other objects (catalogs, etc.), and (to a lesser extent) the hash entries themselves. Even if we were just fulling up a hash with dead-weak-ref => dead-weak-ref pairs, we'd still have a memory leak. I suppose we could do a miniature generational garbage collection scheme, where we periodically copied all the still-valid ones into a new hash, but that seems...counterproductive. And I know testing threading code is always hard but all the refcounting > stuff would benefit from at least a unit test. If possible having an > integration test to check for the thread safeness of the code would be > terrific :) > It should even be possible to test thread safety with a (perhaps convoluted) unit test. As for testing the ref-counting, I would be against it on the "don't test implementation details" principle. We do way too much of that sort of nonsense already IMHO, and it just makes the code brittle and hard to refactor. If we wanted to add tests (which I can see, though I'm not enthusiastic) we should be testing the thread safety and the no-memory-leak aspects, not the internals. > + def self.synchronize_on(x,type) > > + sync_object,users = 0,1 > > Why use variables here for constants? > Doesn't it consume stack space for nothing useful? > A very, very small amount, for a reasonable boost in clarity, which I was more concerned with. I'd also considered using symbols and a two level hash, but this seemed the most straight forward. -- M ----------------------------------------------------------- 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.
