top: remove handle abstraction, use simpler process list

classic Classic list List threaded Threaded
2 messages Options
Reply | Threaded
Open this post in threaded view
|

top: remove handle abstraction, use simpler process list

Klemens Nanni-2
The internal handle used to pass process information is a needless
abstraction, after previously removing an unused member, it now only has
one member pointing to a pointer to a process struct, i.e. a simple list
of processes.

Remove the abstraction layer and (re)use the existing list of
(pointers to) kinfo_proc structs.  The diff is straight forward and I
tested it on amd64 and sparc64.

With this, scrolling does not even deserve its own helper, we can merely
point at the process to format with an offset in the list now.

There's more to clean up, bu this minimal diff already removes 21 LOC.

Feedback? OK?


Index: commands.c
===================================================================
RCS file: /cvs/src/usr.bin/top/commands.c,v
retrieving revision 1.33
diff -u -p -r1.33 commands.c
--- commands.c 8 Oct 2019 07:26:59 -0000 1.33
+++ commands.c 26 Jun 2020 14:35:20 -0000
@@ -36,6 +36,7 @@
  */
 
 #include <sys/types.h>
+#include <sys/sysctl.h>
 #include <stdio.h>
 #include <err.h>
 #include <ctype.h>
Index: display.c
===================================================================
RCS file: /cvs/src/usr.bin/top/display.c,v
retrieving revision 1.61
diff -u -p -r1.61 display.c
--- display.c 27 Oct 2019 13:52:26 -0000 1.61
+++ display.c 26 Jun 2020 14:34:59 -0000
@@ -46,6 +46,7 @@
  */
 
 #include <sys/types.h>
+#include <sys/sysctl.h>
 #include <sys/time.h>
 #include <sys/sched.h>
 #include <curses.h>
Index: machine.c
===================================================================
RCS file: /cvs/src/usr.bin/top/machine.c,v
retrieving revision 1.105
diff -u -p -r1.105 machine.c
--- machine.c 25 Jun 2020 20:38:41 -0000 1.105
+++ machine.c 26 Jun 2020 14:36:25 -0000
@@ -60,12 +60,6 @@ static char *format_comm(struct kinfo_pr
 static int cmd_matches(struct kinfo_proc *, char *);
 static char **get_proc_args(struct kinfo_proc *);
 
-/* get_process_info passes back a handle.  This is what it looks like: */
-
-struct handle {
- struct kinfo_proc **next_proc; /* points to next valid proc pointer */
-};
-
 /* what we consider to be process size: */
 #define PROCSIZE(pp) ((pp)->p_vm_tsize + (pp)->p_vm_dsize + (pp)->p_vm_ssize)
 
@@ -127,7 +121,7 @@ static int nproc;
 static int onproc = -1;
 static int pref_len;
 static struct kinfo_proc *pbase;
-static struct kinfo_proc **pref;
+struct kinfo_proc **pref;
 
 /* these are for getting the memory statistics */
 static int pageshift; /* log base 2 of the pagesize */
@@ -313,8 +307,6 @@ get_system_info(struct system_info *si)
  si->last_pid = -1;
 }
 
-static struct handle handle;
-
 struct kinfo_proc *
 getprocs(int op, int arg, int *cnt)
 {
@@ -413,7 +405,7 @@ cmd_matches(struct kinfo_proc *proc, cha
  return 0;
 }
 
-struct handle *
+void
 get_process_info(struct system_info *si, struct process_select *sel,
     int (*compare) (const void *, const void *))
 {
@@ -489,10 +481,6 @@ get_process_info(struct system_info *si,
  /* remember active and total counts */
  si->p_total = total_procs;
  si->p_active = pref_len = active_procs;
-
- /* pass back a handle */
- handle.next_proc = pref;
- return &handle;
 }
 
 char fmt[MAX_COLS]; /* static area where result is built */
@@ -536,24 +524,14 @@ format_comm(struct kinfo_proc *kp)
  return (buf);
 }
 
-void
-skip_processes(struct handle *hndl, int n)
-{
- hndl->next_proc += n;
-}
-
 char *
-format_next_process(struct handle *hndl, const char *(*get_userid)(uid_t, int),
+format_process(struct kinfo_proc *pp, const char *(*get_userid)(uid_t, int),
     pid_t *pid)
 {
  char *p_wait;
- struct kinfo_proc *pp;
  int cputime;
  double pct;
  char buf[16];
-
- /* find and remember the next proc structure */
- pp = *(hndl->next_proc++);
 
  cputime = pp->p_rtime_sec + ((pp->p_rtime_usec + 500000) / 1000000);
 
Index: machine.h
===================================================================
RCS file: /cvs/src/usr.bin/top/machine.h,v
retrieving revision 1.29
diff -u -p -r1.29 machine.h
--- machine.h 25 Jun 2020 20:38:41 -0000 1.29
+++ machine.h 26 Jun 2020 14:35:56 -0000
@@ -87,11 +87,10 @@ extern int      display_init(struct stat
 extern int      machine_init(struct statics *);
 extern char    *format_header(char *);
 extern void     get_system_info(struct system_info *);
-extern struct handle
-*get_process_info(struct system_info *, struct process_select *,
+extern void
+get_process_info(struct system_info *, struct process_select *,
  int (*) (const void *, const void *));
-extern void     skip_processes(struct handle *, int);
-extern char    *format_next_process(struct handle *,
+extern char    *format_process(struct kinfo_proc *,
  const char *(*)(uid_t, int), pid_t *);
 extern uid_t    proc_owner(pid_t);
 
Index: top.c
===================================================================
RCS file: /cvs/src/usr.bin/top/top.c,v
retrieving revision 1.103
diff -u -p -r1.103 top.c
--- top.c 25 Jun 2020 20:38:41 -0000 1.103
+++ top.c 26 Jun 2020 14:47:42 -0000
@@ -29,6 +29,8 @@
  */
 
 #include <sys/types.h>
+#include <sys/sysctl.h>
+
 #include <curses.h>
 #include <err.h>
 #include <errno.h>
@@ -70,6 +72,8 @@ int rundisplay(void);
 static int max_topn; /* maximum displayable processes */
 static int skip; /* how many processes to skip (scroll) */
 
+extern struct kinfo_proc **pref;
+
 extern int ncpu;
 extern int ncpuonline;
 
@@ -341,7 +345,6 @@ main(int argc, char *argv[])
  int preset_argc = 0, ac = argc, active_procs, i, ncpuonline_now;
  sigset_t mask, oldmask;
  time_t curr_time;
- struct handle *processes;
 
  /* set the buffer for stdout */
 #ifdef DEBUG
@@ -511,8 +514,7 @@ restart:
  }
 
  /* get the current set of processes */
- processes = get_process_info(&system_info, &ps,
-    proc_compares[order_index]);
+ get_process_info(&system_info, &ps, proc_compares[order_index]);
 
  /* display the load averages */
  i_loadave(system_info.last_pid, system_info.load_avg);
@@ -569,13 +571,11 @@ restart:
  */
  if (skip + active_procs > system_info.p_active)
  skip = system_info.p_active - active_procs;
- skip_processes(processes, skip);
- /* now show the top "n" processes. */
  for (i = 0; i < active_procs; i++) {
  pid_t pid;
  char * s;
 
- s = format_next_process(processes,
+ s = format_process((pref + skip)[i],
     ps.threads ? NULL : get_userid, &pid);
  i_process(i, s, pid == hlpid);
  }

Reply | Threaded
Open this post in threaded view
|

Re: top: remove handle abstraction, use simpler process list

Klemens Nanni-2
On Fri, Jun 26, 2020 at 04:48:53PM +0200, Klemens Nanni wrote:

> The internal handle used to pass process information is a needless
> abstraction, after previously removing an unused member, it now only has
> one member pointing to a pointer to a process struct, i.e. a simple list
> of processes.
>
> Remove the abstraction layer and (re)use the existing list of
> (pointers to) kinfo_proc structs.  The diff is straight forward and I
> tested it on amd64 and sparc64.
>
> With this, scrolling does not even deserve its own helper, we can merely
> point at the process to format with an offset in the list now.
Updated diff that uses a `procp' variable in the for loop and does not
rename format_next_process() (separate diff).

Feedback? OK?


Index: commands.c
===================================================================
RCS file: /cvs/src/usr.bin/top/commands.c,v
retrieving revision 1.33
diff -u -p -r1.33 commands.c
--- commands.c 8 Oct 2019 07:26:59 -0000 1.33
+++ commands.c 3 Jul 2020 10:57:15 -0000
@@ -36,6 +36,7 @@
  */
 
 #include <sys/types.h>
+#include <sys/sysctl.h>
 #include <stdio.h>
 #include <err.h>
 #include <ctype.h>
Index: display.c
===================================================================
RCS file: /cvs/src/usr.bin/top/display.c,v
retrieving revision 1.62
diff -u -p -r1.62 display.c
--- display.c 27 Jun 2020 01:09:57 -0000 1.62
+++ display.c 3 Jul 2020 10:57:15 -0000
@@ -46,6 +46,7 @@
  */
 
 #include <sys/types.h>
+#include <sys/sysctl.h>
 #include <sys/time.h>
 #include <sys/sched.h>
 #include <curses.h>
Index: machine.c
===================================================================
RCS file: /cvs/src/usr.bin/top/machine.c,v
retrieving revision 1.106
diff -u -p -r1.106 machine.c
--- machine.c 26 Jun 2020 20:55:55 -0000 1.106
+++ machine.c 3 Jul 2020 11:28:51 -0000
@@ -60,12 +60,6 @@ static char *format_comm(struct kinfo_pr
 static int cmd_matches(struct kinfo_proc *, char *);
 static char **get_proc_args(struct kinfo_proc *);
 
-/* get_process_info passes back a handle.  This is what it looks like: */
-
-struct handle {
- struct kinfo_proc **next_proc; /* points to next valid proc pointer */
-};
-
 /* what we consider to be process size: */
 #define PROCSIZE(pp) ((pp)->p_vm_tsize + (pp)->p_vm_dsize + (pp)->p_vm_ssize)
 
@@ -127,7 +121,7 @@ static int nproc;
 static int onproc = -1;
 static int pref_len;
 static struct kinfo_proc *pbase;
-static struct kinfo_proc **pref;
+struct kinfo_proc **pref;
 
 /* these are for getting the memory statistics */
 static int pageshift; /* log base 2 of the pagesize */
@@ -308,8 +302,6 @@ get_system_info(struct system_info *si)
  si->last_pid = -1;
 }
 
-static struct handle handle;
-
 struct kinfo_proc *
 getprocs(int op, int arg, int *cnt)
 {
@@ -408,7 +400,7 @@ cmd_matches(struct kinfo_proc *proc, cha
  return 0;
 }
 
-struct handle *
+void
 get_process_info(struct system_info *si, struct process_select *sel,
     int (*compare) (const void *, const void *))
 {
@@ -484,10 +476,6 @@ get_process_info(struct system_info *si,
  /* remember active and total counts */
  si->p_total = total_procs;
  si->p_active = pref_len = active_procs;
-
- /* pass back a handle */
- handle.next_proc = pref;
- return &handle;
 }
 
 char fmt[MAX_COLS]; /* static area where result is built */
@@ -531,24 +519,14 @@ format_comm(struct kinfo_proc *kp)
  return (buf);
 }
 
-void
-skip_processes(struct handle *hndl, int n)
-{
- hndl->next_proc += n;
-}
-
 char *
-format_next_process(struct handle *hndl, const char *(*get_userid)(uid_t, int),
+format_next_process(struct kinfo_proc *pp, const char *(*get_userid)(uid_t, int),
     pid_t *pid)
 {
  char *p_wait;
- struct kinfo_proc *pp;
  int cputime;
  double pct;
  char buf[16];
-
- /* find and remember the next proc structure */
- pp = *(hndl->next_proc++);
 
  cputime = pp->p_rtime_sec + ((pp->p_rtime_usec + 500000) / 1000000);
 
Index: machine.h
===================================================================
RCS file: /cvs/src/usr.bin/top/machine.h,v
retrieving revision 1.29
diff -u -p -r1.29 machine.h
--- machine.h 25 Jun 2020 20:38:41 -0000 1.29
+++ machine.h 3 Jul 2020 11:28:54 -0000
@@ -87,11 +87,10 @@ extern int      display_init(struct stat
 extern int      machine_init(struct statics *);
 extern char    *format_header(char *);
 extern void     get_system_info(struct system_info *);
-extern struct handle
-*get_process_info(struct system_info *, struct process_select *,
+extern void
+get_process_info(struct system_info *, struct process_select *,
  int (*) (const void *, const void *));
-extern void     skip_processes(struct handle *, int);
-extern char    *format_next_process(struct handle *,
+extern char    *format_next_process(struct kinfo_proc *,
  const char *(*)(uid_t, int), pid_t *);
 extern uid_t    proc_owner(pid_t);
 
Index: top.c
===================================================================
RCS file: /cvs/src/usr.bin/top/top.c,v
retrieving revision 1.103
diff -u -p -r1.103 top.c
--- top.c 25 Jun 2020 20:38:41 -0000 1.103
+++ top.c 3 Jul 2020 11:28:54 -0000
@@ -29,6 +29,7 @@
  */
 
 #include <sys/types.h>
+#include <sys/sysctl.h>
 #include <curses.h>
 #include <err.h>
 #include <errno.h>
@@ -70,6 +71,8 @@ int rundisplay(void);
 static int max_topn; /* maximum displayable processes */
 static int skip; /* how many processes to skip (scroll) */
 
+extern struct kinfo_proc **pref;
+
 extern int ncpu;
 extern int ncpuonline;
 
@@ -341,7 +344,7 @@ main(int argc, char *argv[])
  int preset_argc = 0, ac = argc, active_procs, i, ncpuonline_now;
  sigset_t mask, oldmask;
  time_t curr_time;
- struct handle *processes;
+ struct kinfo_proc *procp;
 
  /* set the buffer for stdout */
 #ifdef DEBUG
@@ -511,8 +514,7 @@ restart:
  }
 
  /* get the current set of processes */
- processes = get_process_info(&system_info, &ps,
-    proc_compares[order_index]);
+ get_process_info(&system_info, &ps, proc_compares[order_index]);
 
  /* display the load averages */
  i_loadave(system_info.last_pid, system_info.load_avg);
@@ -569,13 +571,12 @@ restart:
  */
  if (skip + active_procs > system_info.p_active)
  skip = system_info.p_active - active_procs;
- skip_processes(processes, skip);
- /* now show the top "n" processes. */
  for (i = 0; i < active_procs; i++) {
  pid_t pid;
  char * s;
 
- s = format_next_process(processes,
+ procp = (pref + skip)[i],
+ s = format_next_process(procp,
     ps.threads ? NULL : get_userid, &pid);
  i_process(i, s, pid == hlpid);
  }