From owner-cvs-all@FreeBSD.ORG Mon Apr 18 10:43:22 2005 Return-Path: Delivered-To: cvs-all@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id 6D5AF16A4CE; Mon, 18 Apr 2005 10:43:22 +0000 (GMT) Received: from kane.otenet.gr (kane.otenet.gr [195.170.0.27]) by mx1.FreeBSD.org (Postfix) with ESMTP id 5DF2B43D53; Mon, 18 Apr 2005 10:43:21 +0000 (GMT) (envelope-from keramida@ceid.upatras.gr) Received: from orion.daedalusnetworks.priv (aris.bedc.ondsl.gr [62.103.39.226])j3IAg9oM020435; Mon, 18 Apr 2005 13:42:13 +0300 Received: from orion.daedalusnetworks.priv (orion [127.0.0.1]) j3IAhEkL013694; Mon, 18 Apr 2005 13:43:14 +0300 (EEST) (envelope-from keramida@ceid.upatras.gr) Received: (from keramida@localhost)j3IAhAY5013693; Mon, 18 Apr 2005 13:43:10 +0300 (EEST) (envelope-from keramida@ceid.upatras.gr) Date: Mon, 18 Apr 2005 13:43:10 +0300 From: Giorgos Keramidas To: Bruce Evans Message-ID: <20050418104310.GA13345@orion.daedalusnetworks.priv> References: <200504161543.j3GFhclO075103@repoman.freebsd.org> <86acnyd2k7.fsf@xps.des.no> <20050416191436.G68941@fledge.watson.org> <863btq4k9m.fsf@xps.des.no> <20050417110321.GA78636@gothmog.gr> <20050417222313.K946@epsplex.bde.org> <20050417135844.GA792@gothmog.gr> <20050418153905.L1289@epsplex.bde.org> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20050418153905.L1289@epsplex.bde.org> cc: Dag-Erling Sm?rgrav cc: src-committers@freebsd.org cc: Robert Watson cc: cvs-all@freebsd.org cc: cvs-src@freebsd.org Subject: Re: cvs commit: src/usr.bin/top machine.c X-BeenThere: cvs-all@freebsd.org X-Mailman-Version: 2.1.1 Precedence: list List-Id: CVS commit messages for the entire tree List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 18 Apr 2005 10:43:22 -0000 On 2005-04-18 15:57, Bruce Evans wrote: >On Sun, 17 Apr 2005, Giorgos Keramidas wrote: >>On 2005-04-17 23:01, Bruce Evans 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