[Crash-utility] [PATCH 3/4] remove unreachable (and slow) code

[Dave Anderson]

----- Original Message -----
> 
> 
> ----- Original Message -----
> > task_exists() scans known tasks for a given task address.
> > 
> > refresh_radix_tree_task_table() looks like:
> >   hq_open()
> >   for cpus {
> >     hq_enter(per_cpu_idle_thread)
> >   }
> >   for ... {
> >     hq_enter(task)
> >   }
> >   hq_close()
> > 
> >   tt->running_tasks = 0
> >   for task in retrieve_list(hq_entries) {
> >     if (task_exists()) {
> >        duplicate task address
> >        continue
> >     }
> >     add_context()
> >   }
> > 
> > Because retrieve_list returns unique tasks, the above task_exists()
> > check will never find a duplicate task.  So remove the check.  This
> > converts the above loop from O(N^2) to O(N) which saves considerable
> > startup time when there are a large number of tasks.  On a 1M task dump
> > this patch reduces startup time: 44m => 1m.
> > 
> > I was not able to test the other pid table routines, but I suspect they
> > may also benefit from similar removal.
> 
> Hi Greg,
> 
> I'm thinking that the task_exists() removal may be the most beneficial
> piece of the patch set, right?
> 
> I have a set of ~250 vmcore files gathered over the years, but only 2 of
> them are from kernels using the idr radix tree scheme.  I timed how long
> it took to simply bring up all 250 vmcores to their crash prompt and then
> quit immediately.  These are the total times running with and without your
> patch:
> 
> without the patch:
> 
>   real	46m44.023s
>   user	17m29.956s
>   sys	0m58.940s
> 
> with your patch applied:
> 
>   real	46m39.752s
>   user	17m36.496s
>   sys	        0m58.269s
> 
> But as you've noted, given that all of the various refresh_xxx functions
> use hq_enter(), the task_exists() call is pretty much useless.
> 
> I'm going to remove the task_exists() call from the remaining refresh_xxx
> functions, rerun the test, and see how it affects the numbers.  I suspect
> it will help a bit, although the task count in the ~250 vmcores range from
> 22 to 4178, with only ~20 dumpfiles having more than 1000 tasks.
> 
> Thanks,
>   Dave

As it turns out, there wasn't any noticeable benefit with my set of dumpfiles:

without the patch:

  real	46m58.872s
  user	17m43.273s
  sys	1m0.144s

with your patch plus all task_exists() calls removed:

  real	46m58.617s
  user	17m50.023s
  sys	0m58.708s

But obviously the extremely large task count in your dumpfile shows the
benefit.  In addition to the task_exists() removals, I also need to update
dump_task_table() to add a display of the context_by_task array (for help -T),
and the INDEXED_CONTEXTS flag (for help -[tT]).  When I get it checked in to
github, I'll post the link here as confirmation.

Thanks,
  Dave