Bug 47053

On Tue Aug 27 2002 at 12:09:35PM -0000 drehamrad@yahoo.de wrote:
Hi

> It would be nice to have a view/sensor for only one task/process tree.
You mean a ProcessView where only one special process is shown?

Ciao
Tobias
-- 
In a world without walls and fences who
needs Windows and Gates???

Correct. I want to observe one or only chosen processes. In the strict
sense I want to observe all threads of my application.

----- Original Message -----
From: "Tobias Koenig" <tokoe@kde.org>
To: <47053@bugs.kde.org>
Sent: Tuesday August 27 2002 7:42 PM
Subject: Bug#47053: View of one task


> On Tue Aug 27 2002 at 12:09:35PM -0000 drehamrad@yahoo.de wrote:
> Hi
>
> > It would be nice to have a view/sensor for only one task/process tree.
> You mean a ProcessView where only one special process is shown?
>
> Ciao
> Tobias
> --
> In a world without walls and fences who
> needs Windows and Gates???
>
> (Complete bug history is available at
http://bugs.kde.org/db/47/47053.html)

__________________________________________________________________

Gesendet von Yahoo! Mail - http://mail.yahoo.de
Möchten Sie mit einem Gruà antworten? http://grusskarten.yahoo.de

On Thu Aug 29 2002 at 10:29:20AM +0200 drehamrad wrote:
> Correct. I want to observe one or only chosen processes.
Hmm then you have to chose the process number at every start of ksysguard.

> In the strict sense I want to observe all threads of my application.
Threads aren't displayed by the kernel in the proc file system so we can't
provide thread informations in ksysguard.

Ciao
Tobias
-- 
In a world without walls and fences who
needs Windows and Gates???

> On Thu Aug 29 2002 at 10:29:20AM +0200 drehamrad wrote:
> > Correct. I want to observe one or only chosen processes.
> Hmm then you have to chose the process number at every start of
ksysguard.
Yes. Or select some process numbers which not to be shown. I think it isn't
necessarily to
store this settings permanently.
>
> > In the strict sense I want to observe all threads of my application.
> Threads aren't displayed by the kernel in the proc file system so we
can't
> provide thread informations in ksysguard.
It may be possible that it's true for other operating systems but not for
Linux.
Linux treats threads as processes (child processes) now you have all
informations of threads.(please take a look at the tree view of process
table)

cu sem+carina

__________________________________________________________________

Gesendet von Yahoo! Mail - http://mail.yahoo.de
Möchten Sie mit einem Gruà antworten? http://grusskarten.yahoo.de

On Fri Aug 30 2002 at 12:00:07PM +0200 drehamrad wrote:
Hi

> > Hmm then you have to chose the process number at every start of ksysguard.
> Yes. Or select some process numbers which not to be shown. I think it isn't
> necessarily to store this settings permanently.
So we could extend the combobox to provide input a process number that should
be shown.

> > Threads aren't displayed by the kernel in the proc file system so we can't
> > provide thread informations in ksysguard.
> It may be possible that it's true for other operating systems but not for
> Linux.
> Linux treats threads as processes (child processes) now you have all
> informations of threads.(please take a look at the tree view of process
> table)
But how to differ threads from child processes?

Ciao
Tobias
-- 
In a world without walls and fences who
needs Windows and Gates???

Created attachment 14088 [details]
Needs a lot of beautification, but it seems to work

Hopefully someone can go over this and find all of my mistakes :)

It's in pretty bad shape right now, but I'll be putting some more work into it.


I would appreciate any feedback.

Created attachment 14089 [details]
Proposed patch in action

Created attachment 14092 [details]
slightly better patch

OK, sorry about taking some time to review, but I've gone over it a bit.

Overall, the logic appears to be good.  I've got a few quibbles though:

1. You mentioned that the stat.c file needs to be split up.  That is certainly true, so why don't you take the opportunity to factor out your changes into a different file.  Or, you may want to consider taking a look at ProcessList.{h,c}, which seems to do a lot of what your code is doing (of course, that's just to list procs, not focus in on a single one).  Perhaps it would be better to merge with ProcessList.  If you do create a different file, you can update ksysguardd/modules.h with the relevant files and functions.

2. The ProcessInfo struct that you have defined, and the associated code to read the ProcessInfo from /proc/pid/stat, seems to go way overboard.  Does the struct really need to read in all 37 entries to get the info that is needed?  In ProcessList.c, there is code that has to read from /proc/pid/stat, which is much shorter because only the parts that are used are picked out.  Also, the stime and nice variable names generate warnings on gcc because they hide corresponding function names.

3. There is some kind of C container library code in ksysguard/CContLib/ which you may want to look at.  Your C linked list code appears to be correct, but since I haven't touched that kind of stuff in years you may want to use the container library.  Of course, then we'll lose type safety because the backends are in C. :(  It's your call.

4. The naming of the fooPROCx() functions seems awkward.  e.g. I'd recommend processIndividualProc() as opposed to processPROCx().

5. The reading of the /proc directory in initStat():  Does that actually register a monitor for every process?  Because running the code causes a noticeable pause bringing up the sensor browser, and it's not that easy to pick out the right PID anyways. :(

I guess it's not your fault because the interface design of ksysguard makes it hard to be able to just right-click on the process in the Process List to get the graph.  But as it stands it's still not very useful because it's hard to pick the right process.

6. I get this warning while compiling: /home/kde-cvs/kdesvn/kdebase/ksysguard/ksysguardd/Linux/stat.c:1518: warning: use of assignment suppression and length modifier together in scanf format

7. The units show up as Hz.  Shouldn't it be % like the other Load values?

I know it seems that I'm splitting hairs but really the code is mostly good. :-)  And it seems to work, to boot.

Has that patch been fixed and applied? If not, can it be ported to KDE 4?

*** This bug has been marked as a duplicate of bug 127728 ***