Re: [PATCH] Make top -P an interactive toggle

On Fri Jul  8 11, John Baldwin wrote:
> This patch lets you use 'P' while top is running to toggle between per-CPU and
> global CPU stats.

very cool. i always thought that being able to interactivly enable/disable
per-cpu stats in top would be a useful feature. great to see this being
implemented.

top is nearing perfection. ;)

> 
> Index: contrib/top/top.c
> ===================================================================
> --- contrib/top/top.c	(revision 223873)
> +++ contrib/top/top.c	(working copy)
> _at__at_ -196,9 +196,9 _at__at_
>      fd_set readfds;
>  
>  #ifdef ORDER
> -    static char command_chars[] = "\f qh?en#sdkriIutHmSCajzo";
> +    static char command_chars[] = "\f qh?en#sdkriIutHmSCajzPo";
>  #else
> -    static char command_chars[] = "\f qh?en#sdkriIutHmSCajz";
> +    static char command_chars[] = "\f qh?en#sdkriIutHmSCajzP";
>  #endif
>  /* these defines enumerate the "strchr"s of the commands in command_chars */
>  #define CMD_redraw	0
> _at__at_ -225,8 +225,9 _at__at_
>  #define	CMD_showargs	20
>  #define	CMD_jidtog	21
>  #define CMD_kidletog	22
> +#define CMD_pcputog	23
>  #ifdef ORDER
> -#define CMD_order       23
> +#define CMD_order       24
>  #endif
>  
>      /* set the buffer for stdout */
> _at__at_ -411,7 +412,7 _at__at_
>  		break;
>  
>  	      case 'P':
> -		pcpu_stats = Yes;
> +		pcpu_stats = !pcpu_stats;
>  		break;
>  
>  	      case 'z':
> _at__at_ -1088,6 +1089,12 _at__at_
>  				    ps.kidle ? "D" : "Not d");
>  				putchar('\r');
>  				break;
> +			    case CMD_pcputog:
> +				pcpu_stats = !pcpu_stats;
> +				toggle_pcpustats(&statics);
> +				max_topn = display_updatecpus(&statics);
> +				reset_display();
> +				break;
>  			    default:
>  				new_message(MT_standout, " BAD CASE IN SWITCH!");
>  				putchar('\r');
> Index: contrib/top/display.c
> ===================================================================
> --- contrib/top/display.c	(revision 223873)
> +++ contrib/top/display.c	(working copy)
> _at__at_ -151,16 +151,14 _at__at_
>      return(smart_terminal ? lines : Largest);
>  }
>  
> -int display_init(statics)
> +int display_updatecpus(statics)
>  
>  struct statics *statics;
>  
>  {
>      register int lines;
> -    register char **pp;
> -    register int *ip;
>      register int i;
> -
> +    
>      /* call resize to do the dirty work */
>      lines = display_resize();
>      num_cpus = statics->ncpus;
> _at__at_ -170,6 +168,21 _at__at_
>      for (i = num_cpus; i > 9; i /= 10)
>  	cpustates_column++;
>  
> +    return(lines);
> +}
> +    
> +int display_init(statics)
> +
> +struct statics *statics;
> +
> +{
> +    register int lines;
> +    register char **pp;
> +    register int *ip;
> +    register int i;
> +
> +    lines = display_updatecpus(statics);
> +
>      /* only do the rest if we need to */
>      if (lines > -1)
>      {
> Index: contrib/top/top.X
> ===================================================================
> --- contrib/top/top.X	(revision 223873)
> +++ contrib/top/top.X	(working copy)
> _at__at_ -205,6 +205,7 _at__at_
>  .BR \-H ,
>  .BR \-I ,
>  .BR \-j ,
> +.BR \-P ,
>  .BR \-S ,
>  .BR \-t ,
>  .BR \-u ,
> _at__at_ -314,6 +315,9 _at__at_
>  .IR jail (8)
>  ID.
>  .TP
> +.B P
> +Toggle the display of per-CPU statistics.
> +.TP
>  .B t
>  Toggle the display of the
>  .I top
> Index: usr.bin/top/machine.c
> ===================================================================
> --- usr.bin/top/machine.c	(revision 223873)
> +++ usr.bin/top/machine.c	(working copy)
> _at__at_ -239,19 +239,48 _at__at_
>  static void getsysctl(const char *name, void *ptr, size_t len);
>  static int swapmode(int *retavail, int *retfree);
>  
> +void
> +toggle_pcpustats(struct statics *statics)
> +{
> +
> +	if (ncpus == 1)
> +		return;
> +
> +	/* Adjust display based on ncpus */
> +	if (pcpu_stats) {
> +		y_mem += ncpus - 1;	/* 3 */
> +		y_swap += ncpus - 1;	/* 4 */
> +		y_idlecursor += ncpus - 1; /* 5 */
> +		y_message += ncpus - 1;	/* 5 */
> +		y_header += ncpus - 1;	/* 6 */
> +		y_procs += ncpus - 1;	/* 7 */
> +		Header_lines += ncpus - 1; /* 7 */
> +		statics->ncpus = ncpus;
> +	} else {
> +		y_mem = 3;
> +		y_swap = 4;
> +		y_idlecursor = 5;
> +		y_message = 5;
> +		y_header = 6;
> +		y_procs = 7;
> +		Header_lines = 7;
> +		statics->ncpus = 1;
> +	}
> +}
> +
>  int
>  machine_init(struct statics *statics, char do_unames)
>  {
> -	int pagesize;
> -	size_t modelen;
> +	int i, j, empty, pagesize;
> +	size_t size;
>  	struct passwd *pw;
>  
> -	modelen = sizeof(smpmode);
> -	if ((sysctlbyname("machdep.smp_active", &smpmode, &modelen,
> +	size = sizeof(smpmode);
> +	if ((sysctlbyname("machdep.smp_active", &smpmode, &size,
>  	    NULL, 0) != 0 &&
> -	    sysctlbyname("kern.smp.active", &smpmode, &modelen,
> +	    sysctlbyname("kern.smp.active", &smpmode, &size,
>  	    NULL, 0) != 0) ||
> -	    modelen != sizeof(smpmode))
> +	    size != sizeof(smpmode))
>  		smpmode = 0;
>  
>  	if (do_unames) {
> _at__at_ -299,52 +328,38 _at__at_
>  	statics->order_names = ordernames;
>  #endif
>  
> -	/* Adjust display based on ncpus */
> -	if (pcpu_stats) {
> -		int i, j, empty;
> -		size_t size;
> -
> -		cpumask = 0;
> -		ncpus = 0;
> -		GETSYSCTL("kern.smp.maxcpus", maxcpu);
> -		size = sizeof(long) * maxcpu * CPUSTATES;
> -		times = malloc(size);
> -		if (times == NULL)
> -			err(1, "malloc %zd bytes", size);
> -		if (sysctlbyname("kern.cp_times", times, &size, NULL, 0) == -1)
> -			err(1, "sysctlbyname kern.cp_times");
> -		pcpu_cp_time = calloc(1, size);
> -		maxid = (size / CPUSTATES / sizeof(long)) - 1;
> -		for (i = 0; i <= maxid; i++) {
> -			empty = 1;
> -			for (j = 0; empty && j < CPUSTATES; j++) {
> -				if (times[i * CPUSTATES + j] != 0)
> -					empty = 0;
> -			}
> -			if (!empty) {
> -				cpumask |= (1ul << i);
> -				ncpus++;
> -			}
> +	/* Allocate state for per-CPU stats. */
> +	cpumask = 0;
> +	ncpus = 0;
> +	GETSYSCTL("kern.smp.maxcpus", maxcpu);
> +	size = sizeof(long) * maxcpu * CPUSTATES;
> +	times = malloc(size);
> +	if (times == NULL)
> +		err(1, "malloc %zd bytes", size);
> +	if (sysctlbyname("kern.cp_times", times, &size, NULL, 0) == -1)
> +		err(1, "sysctlbyname kern.cp_times");
> +	pcpu_cp_time = calloc(1, size);
> +	maxid = (size / CPUSTATES / sizeof(long)) - 1;
> +	for (i = 0; i <= maxid; i++) {
> +		empty = 1;
> +		for (j = 0; empty && j < CPUSTATES; j++) {
> +			if (times[i * CPUSTATES + j] != 0)
> +				empty = 0;
>  		}
> -
> -		if (ncpus > 1) {
> -			y_mem += ncpus - 1;	/* 3 */
> -			y_swap += ncpus - 1;	/* 4 */
> -			y_idlecursor += ncpus - 1; /* 5 */
> -			y_message += ncpus - 1;	/* 5 */
> -			y_header += ncpus - 1;	/* 6 */
> -			y_procs += ncpus - 1;	/* 7 */
> -			Header_lines += ncpus - 1; /* 7 */
> +		if (!empty) {
> +			cpumask |= (1ul << i);
> +			ncpus++;
>  		}
> -		size = sizeof(long) * ncpus * CPUSTATES;
> -		pcpu_cp_old = calloc(1, size);
> -		pcpu_cp_diff = calloc(1, size);
> -		pcpu_cpu_states = calloc(1, size);
> -		statics->ncpus = ncpus;
> -	} else {
> -		statics->ncpus = 1;
>  	}
> +	size = sizeof(long) * ncpus * CPUSTATES;
> +	pcpu_cp_old = calloc(1, size);
> +	pcpu_cp_diff = calloc(1, size);
> +	pcpu_cpu_states = calloc(1, size);
> +	statics->ncpus = 1;
>  
> +	if (pcpu_stats)
> +		toggle_pcpustats(statics);
> +
>  	/* all done! */
>  	return (0);
>  }
> _at__at_ -398,14 +413,11 _at__at_
>  	int i, j;
>  	size_t size;
>  
> -	/* get the cp_time array */
> -	if (pcpu_stats) {
> -		size = (maxid + 1) * CPUSTATES * sizeof(long);
> -		if (sysctlbyname("kern.cp_times", pcpu_cp_time, &size, NULL, 0) == -1)
> -			err(1, "sysctlbyname kern.cp_times");
> -	} else {
> -		GETSYSCTL("kern.cp_time", cp_time);
> -	}
> +	/* get the CPU stats */
> +	size = (maxid + 1) * CPUSTATES * sizeof(long);
> +	if (sysctlbyname("kern.cp_times", pcpu_cp_time, &size, NULL, 0) == -1)
> +		err(1, "sysctlbyname kern.cp_times");
> +	GETSYSCTL("kern.cp_time", cp_time);
>  	GETSYSCTL("vm.loadavg", sysload);
>  	GETSYSCTL("kern.lastpid", lastpid);
>  
> _at__at_ -413,21 +425,17 _at__at_
>  	for (i = 0; i < 3; i++)
>  		si->load_avg[i] = (double)sysload.ldavg[i] / sysload.fscale;
>  
> -	if (pcpu_stats) {
> -		for (i = j = 0; i <= maxid; i++) {
> -			if ((cpumask & (1ul << i)) == 0)
> -				continue;
> -			/* convert cp_time counts to percentages */
> -			percentages(CPUSTATES, &pcpu_cpu_states[j * CPUSTATES],
> -			    &pcpu_cp_time[j * CPUSTATES],
> -			    &pcpu_cp_old[j * CPUSTATES],
> -			    &pcpu_cp_diff[j * CPUSTATES]);
> -			j++;
> -		}
> -	} else {
> -		/* convert cp_time counts to percentages */
> -		percentages(CPUSTATES, cpu_states, cp_time, cp_old, cp_diff);
> +	/* convert cp_time counts to percentages */
> +	for (i = j = 0; i <= maxid; i++) {
> +		if ((cpumask & (1ul << i)) == 0)
> +			continue;
> +		percentages(CPUSTATES, &pcpu_cpu_states[j * CPUSTATES],
> +		    &pcpu_cp_time[j * CPUSTATES],
> +		    &pcpu_cp_old[j * CPUSTATES],
> +		    &pcpu_cp_diff[j * CPUSTATES]);
> +		j++;
>  	}
> +	percentages(CPUSTATES, cpu_states, cp_time, cp_old, cp_diff);
>  
>  	/* sum memory & swap statistics */
>  	{
> 
> -- 
> John Baldwin