Hi, On Fri, Mar 13, 2009 at 05:52:07PM +0000, Jeremy Henty wrote:
On Fri, Mar 13, 2009 at 08:40:45AM -0300, Jorge Arellano Cid wrote:
Anyway I reviewed the patch back then, and remembered that the current interface was meant for flexibility to experiment.
Yes, there is a case for keeping the flags for future development. I just wanted to point that they are currently redundant and we could remove them if we wanted.
I'd prefer to keep the flags.
I have to give it a second review. Anyway, if there're more cleanups pending on this patch, it may be worth accepting them
Just so you know, the cleanups I have pending refactor a lot of code that calls the dList_* API in convoluted and inefficient ways. (Most stuff that call dList_remove() are like this.) You get cleaner and more efficient code by defining new dList_remove_* and dList_free_* functions, because that lets you replace lots of convoluted code paths that call dList_remove(), dList_nth_data() etc. with single API calls.
dList API was designed with a minimal working function set in mind. This keeps the API easy to remmeber and use. Now, if there's a measurable performance gain with the added functions it may be worth adding them.
One function that gets refactored is a_Bw_stop_clients(), which is why submitting the cleanup patches is waiting on the decision about the BM_Flags patch. It won't be hard to fix the cleanup patches if the BM_Flags patch does not go in, but until the decision is made I don't know whether or not to do that.
Please try to get some performance numbers, and in the meanwhile send me a summary of the API changes. -- Cheers Jorge.-