cvs commit: src/usr.bin/top machine.c

[Giorgos Keramidas]

On 2005-04-18 15:57, Bruce Evans <bde at zeta.org.au> wrote:
>On Sun, 17 Apr 2005, Giorgos Keramidas wrote:
>>On 2005-04-17 23:01, Bruce Evans <bde at zeta.org.au> wrote:
>>> Just removing the CPU column doesn't seem right.  ps has a -C flag
>>> to switch between WCPU and CPU.
>>
>> We can add a new toggle command to top, i.e. 'C', that toggles
>> between displaying WCPU or CPU.  This should regain us a lot of the
>> space lost from COMMAND after I added THR.
>
> I like that.

Unfortunately, David has already said he doesn't, so making such a
chance wouldn't be good enough, but thanks :)

>> --- contrib/top/commands.c	14 Apr 2005 15:02:03 -0000	1.11
>> +++ contrib/top/commands.c	17 Apr 2005 13:53:19 -0000
>> @@ -71,6 +71,7 @@
>> d       - change number of displays to show\n\
>> e       - list errors generated by last \"kill\" or \"renice\" command\n\
>> i or I  - toggle the displaying of idle processes\n\
>> +C       - toggle the displaying of weighted CPU percentage\n\
>> H       - toggle the displaying of threads\n\
>> k       - kill processes; send a signal to a list of processes\n\
>> m       - toggle the display between 'cpu' and 'io' modes\n\
>
> Further unsorting of a sorted list.  The list was previously unsorted by
> adding H after I.

I'll add the C option to the right place in my local copy.  Moving H too
is an option, but that should probably be done as part of a different
style-sweep of top, which has an amazing number of style bugs already.

>> --- contrib/top/machine.h	17 Jul 2003 23:56:40 -0000	1.6
>> +++ contrib/top/machine.h	17 Apr 2005 13:37:40 -0000
>> @@ -60,6 +60,7 @@
>>     int self;		/* show self */
>>     int system;		/* show system processes */
>>     int thread;		/* show threads */
>> +    int wcpu;		/* show weighted cpu */
>>     int uid;		/* only this uid (unless uid == -1) */
>>     char *command;	/* only this command (unless == NULL) */
>> };
>
> More unsorting of a sorted list.

Fixed, thanks :-)

>> Index: contrib/top/top.c
>> ===================================================================
>> RCS file: /home/ncvs/src/contrib/top/top.c,v
>> retrieving revision 1.16
>> diff -u -r1.16 top.c
>> --- contrib/top/top.c	14 Apr 2005 15:02:03 -0000	1.16
>> +++ contrib/top/top.c	17 Apr 2005 13:43:26 -0000
>> @@ -193,9 +193,9 @@
>>     fd_set readfds;
>>
>> #ifdef ORDER
>> -    static char command_chars[] = "\f qh?en#sdkriIutHmSo";
>> +    static char command_chars[] = "\f qh?en#sdkriIutHCmSo";
>> #else
>> -    static char command_chars[] = "\f qh?en#sdkriIutHmS";
>> +    static char command_chars[] = "\f qh?en#sdkriIutHCmS";
>> #endif
>
> Unsorting of unsorted lists.

These arrays are used as arrays of characters matching the CMD_xxx
options.  I'm not sure if it is possible to keep both the list of
CMD_xxx defines *and* the characters in command_chars[] sorted, or
if it's worth the extra trouble.

>> /* these defines enumerate the "strchr"s of the commands in command_chars
>>  */
>>  #define CMD_redraw	0
>>  @@ -216,10 +216,11 @@
>>  #define CMD_user	14
>>  #define CMD_selftog	15
>>  #define CMD_thrtog	16
>> -#define CMD_viewtog	17
>> -#define CMD_viewsys	18
>> +#define	CMD_wcputog	17
>> +#define CMD_viewtog	18
>> +#define CMD_viewsys	19
>>  #ifdef ORDER
>> -#define CMD_order       19
>> +#define CMD_order       20
>>  #endif
> >
> >    /* set the buffer for stdout */
>
> Adding at the end of this list would be acceptable, to avoid
> renumbering everything.  But iterations of that is apparently
> how the order of command_chars[] became random (historical).

I know :-/

The diffs of the changes that touch this list seem to insert the option
somewhere and then renumber everything as needed.  I don't like this,
but it seems to be the way it's been done since practically forever.

> >--- usr.bin/top/machine.c	16 Apr 2005 15:43:38 -0000	1.71
> >+++ usr.bin/top/machine.c	17 Apr 2005 13:51:12 -0000
>
> I thought that you would have to change the sorting method to switch
> between WCPU and CPU, but the existing sorting is only on CPU.  It is
> just more bogus to sort on CPU while only displaying WCPU.

Ah, yes.  Good point.  I thought about it for a while, but was
undecided.  Changing the sort order needs a new sorting method,
but it's definitely something I can easily add.

I'm not sure if I got all the details right, but changing ORDER_PCTCPU
seems to be the easiest way to do this without having to introduce
changes to a lot of places.  The top.wcputog2.diff diff at my personal
space on freefall does this, with a modification to the macro:
http://people.freebsd.org/~keramida/diff/top.wcputog2.diff

%  #define ORDERKEY_PCTCPU(a, b) do { \
% -       long diff = (long)(b)->ki_pctcpu - (long)(a)->ki_pctcpu; \
% +       long diff; \
% +       if (ps.wcpu) \
% +               diff = floor(100.0 * weighted_cpu(pctdouble((a)->ki_pctcpu), (b))) - \
% +                   floor(100.0 * weighted_cpu(pctdouble((a)->ki_pctcpu), (a))); \
% +       else \
% +               diff = (long)(b)->ki_pctcpu - (long)(a)->ki_pctcpu; \
%         if (diff != 0) \
%                 return (diff > 0 ? 1 : -1); \
%  } while (0)

> >@@ -780,8 +780,7 @@
> >	    status,
> >	    smpmode ? pp->ki_lastcpu : 0,
> >	    format_time(cputime),
> >-	    100.0 * weighted_cpu(pct, pp),
> >-	    100.0 * pct,
> >+	    ps.wcpu ? (100.0 * weighted_cpu(pct, pp)) : (100.0 * pct),
>
> Excessive parentheses.

Removed, thanks :)

- Giorgos