Re: code cleanup

John Baldwin <jhb_at_FreeBSD.org> wrote:
> On Thursday 29 April 2004 12:06 am, Alex Lyashkov wrote:
> > > Note that the allproc_lock protects the allproc list.  W/o the
> > > FOREACH_PROC macro, I can grep for 'allproc' in the source tree to find
> > > all users to verify locking, etc.  With the extra macro, I now have to do
> > > multiple greps.
> >
> > two greps is multiple ? first of FOREACH_PROC, second allproc or combine
> > at one grep with two -e parameters.
> 
> Multiple means more than one, yes.  When I'm searching the tree when locking a 
> structure or fields of a structure I don't usually come up with complex grep 
> statements, and actually, I wouldn't find the FOREACH_FOO macro until I did 
> the first grep anyway.  When you add lots of macros that do this you get a 
> compounding problem.

For what it's worth, I don't think it is good to hide things as much as
FOREACH_PROC_IN_SYSTEM() -- this specific instance -- does, but grep is not 
a good tool for a tree as large as FreeBSD's.  Try using cscope instead.

(This next part is slow.)
$ cscope -qRb           
(This next part is BLAZING FAST, even with MUTEX_PROFILING.)
$ cscope -qRdL -0 allproc
kern/kern_proc.c <global> 89 struct proclist allproc;
sys/proc.h <global> 811 extern struct proclist allproc;
alpha/alpha/db_trace.c db_stack_trace_cmd 253 LIST_FOREACH(p, &allproc, p_list) {
...
sys/proc.h FOREACH_PROC_IN_SYSTEM 682 LIST_FOREACH((p), &allproc, p_list)
(There's the hidden allproc.  Recursing to find the hidden usage is also 
blazing fast.)
$ cscope -qRdL -0 FOREACH_PROC_IN_SYSTEM
sys/proc.h <global> 681 #define FOREACH_PROC_IN_SYSTEM(p) \
kern/kern_resource.c setpriority 241 FOREACH_PROC_IN_SYSTEM(p) {
kern/sched_4bsd.c schedcpu 296 FOREACH_PROC_IN_SYSTEM(p) {
kern/subr_witness.c DB_SHOW_COMMAND 1833 FOREACH_PROC_IN_SYSTEM(p) {
vm/vm_glue.c vm_proc_swapin_all 389 FOREACH_PROC_IN_SYSTEM(p) {
vm/vm_glue.c scheduler 811 FOREACH_PROC_IN_SYSTEM(p) {
vm/vm_glue.c swapout_procs 919 FOREACH_PROC_IN_SYSTEM(p) {
vm/vm_meter.c vmtotal 115 FOREACH_PROC_IN_SYSTEM(p) {
vm/vm_pageout.c vm_pageout_scan 1171 FOREACH_PROC_IN_SYSTEM(p) {

The macros are obtuse, but using grep wastes time, too.  Please consider the 
alternatives instead.  Here's an example of how much time you can save:

$ alias cs="cscope -qR"
$ alias csupdate="cs -b"    
$ alias csgrep="cs -dL -0"
$ rm /home/green/cscope.*
$ time csupdate # first build
  125.89s real    22.76s user    13.11s system
$ time csupdate # only looking for updates
   11.75s real     0.18s user     2.22s system
$ time csgrep vmspace_dofree              
vm/vm_map.c vmspace_dofree 282 vmspace_dofree(struct vmspace *vm)
vm/vm_map.c vmspace_free 316 vmspace_dofree(vm);
vm/vm_map.c vmspace_exitfree 340 vmspace_dofree(vm);
    0.06s real     0.00s user     0.02s system


-- 
Brian Fundakowski Feldman                           \'[ FreeBSD ]''''''''''\
  <> green_at_FreeBSD.org                               \  The Power to Serve! \
 Opinions expressed are my own.                       \,,,,,,,,,,,,,,,,,,,,,,\