On Tue, 2011-08-30 at 17:54 +0200, Jan Zelený wrote:
Attached patch rewrites almost entire memberof plugin. It heavily
utilizes
hash tables instead of lists and arrays and it introduces concept of reference
counting which should heavily optimize all operations when no loops are
present in the user/group tree.
I tested the patch by running sysdb test suite, all tests passed. I also
attach a document where basic concepts of the plugin are explained.
Please note that the patch is functional, although I don't consider it ready.
I just want to get pre-ACK or some comments about the patch design. I have yet
to implement the recompute task, I will work on that later.
Jan,
yesterday I an Stephen got together to start a review of the patch. Due
to the complexity we tried to go thorugh the code together to make sure
we understand what is going on.
Also due to the complexity and need to refresh memory we were only be
able to go through most of the "add" code only, we were not able to
review the update or delete code paths yet.
Attached find a diff file with comments in C++ style (so that it is
clear they all need to be resolved before the patch can be considered
ok, as they show as a big red line in our editors) that you can
integrate in your tree and check one by one.
There are some questions in there, but in general we noticed a very
annoying lack of comments in the code itself. The accompaning PDF is
nice and all, but unless the logic is embedded and explained in the code
(like it was with the previous code) it will be ablocker and will make
review more difficult.
That said, so far so good, although we need to more carefully review the
refcounting stuff, we got to it late and only marginally reviewed it.
Need more time to fully grasp all details.
HTH,
Simo.
--
Simo Sorce * Red Hat, Inc * New York