On Thu, Jan 21, 2010 at 05:33:04PM -0600, Shawn Walker wrote: > On 01/21/10 05:13 PM, [email protected] wrote: > >Folks, > >I have a small webrev that fixes a pair of locking bugs, as well as a > >performance problem that Danek found on Friday. > > > >These are the bugs: > > > > 3720 Race condition exists for cancelation using the API > > 13925 installs slow down towards the end > > 14049 Classes that handle RLock have trouble with NRLock > > > >This is the webrev: > > > > http://cr.opensolaris.org/~johansen/webrev-3720/ > > Seems reasonable, although I wonder if the changes for 13925 made > any measurable impact on our current RSS usage?
I wasn't able to measure any substantive difference. I added the call to cleanup the dictionary mostly to help the GUI, which can run multiple installs in a single address space. It seemed better to clean this up, even if they eventually throw away the imageplan later on. > As for the cancel changes, the only comment I have there is that it > seems like (excluding the handler/cleanup functions in the API) > every caller that wants to enable cancel has to through the > acquire/release lock dance. > > Perhaps we should just have a __enable_cancel function as we already > have a __disable_cancel that performs the same task? Or consolidate > them somehow into __set_can_be_canceled or something else? We could. The __disable_cancel function is there because the disable logic is a bit more complicated. If we're trying to disable cancellation, but one has already been requested, then we need to raise the exception so callers that are expecting to catch the exception and take action still can. While writing this paragraph, I noticed that not all callers of __disable_cancel do this yet, so I'll need to make a change in the API to accomodate such a scenario. The enable logic is pretty straight forward, and all cases are the same. We could turn this into a function, but then we're adding another stack frame for a pretty simple task. -j _______________________________________________ pkg-discuss mailing list [email protected] http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
