[Toybox] ps down, top to go

[enh]

On Fri, May 20, 2016 at 3:37 PM, Rob Landley <rob at landley.net> wrote:
> On 05/20/2016 04:57 PM, enh wrote:
>> On Fri, May 20, 2016 at 2:30 PM, Rob Landley <rob at landley.net> wrote:
>>> Threads have nothing in /proc/$$/cmdline, so command and friends have
>>> nothing to show and fall back to showing kernel thread, yes.
>>>
>>> It sounds like the behavior you _want_ is for one of them to show the
>>> $PID command line for this $TID? I.E. show some OTHER process's command
>>> line because threads have magic relationships that ps needs to learn
>>> about. Most likely this should be CMDLINE doing it.
>>
>> yes.
>>
>> for us, where everything's threaded, it's getpid() == gettid() that's
>> the weird special case.
>
> I'm pretty sure every thread group should have one of those, so it's not
> THAT much of a special case.
>
> Digging into the code get_ps takes a struct dirtree * which has a
> ->parent pointing to the base process for the threads, so I actually
> _do_ have access to that info and can just snapshot it at get time.
>
> The "replace slot -1" thing is easiest to implement, and since the zero
> length command line stored the same information as pid != tid (I.E.
> Admiral Ackbar telling us "it's a thread!"), no actual data is lost.
>
> I think if I do that, I still want COMM to stick [brackets] around
> thread names because it makes 'em stand out more, but I can add a tid !=
> pid test for that. (And keep the old one for kernel threads, which have
> no parent.)
>
>>> So, what we gotta do is snapshot the data into toybuf. I can add another
>>> entry to the fetch[] array at the start of get_ps() and have that be a
>>> zero length string for non-threads but a copy of the parent process's
>>> command line for threads, and then have CMDLINE print that if it's
>>> non-null, otherwise fall back to previous behavior. Actually I can be
>>> slimy about initializing struct carveup offset[6] so it only points to
>>> the new entry if there is one, and points to
>>
>> assuming you were going to finish that sentence more reasonably than
>> "yo momma", yes, that sounds like what toolbox did.
>
> I think I figured out a simpler way to do it. :)
>
>>> The question is, which -o fields should show slot -7 and which should
>>> show slot -1? I.E. when do I do the current [thread] behavior, and when
>>> do I lie and show the parent's command line instead?
>>
>> i think that your existing heuristics are fine for ps. i think it's
>> only top (where you lose the relationship between pid and tids) where
>> it matters.
>
> Top defaults to showing args, not cmd/comm, so under the new way of
> doing stuff threads would get command lines there by default.
>
>> toolbox top wasn't trying to be consistent with ps, and
>> neither supported all the traditional CMD/COMM/CMDLINE/ARGS variants,
>> so effectively just went with a new field that's explicitly the
>> process name. so if you don't mind adding yet another field name, i
>> think that's the simplest option (and simplest to explain in the
>> --help output too).
>
> Or I can do that, but when you say "process name" do you mean:
>
> 1) stat[2]
> 2) argv[0] with path
> 3) argv[0] minus path
> 4) argv[]
> 5) something else?

/me checks the source... looks like /proc/%d/cmdline up to the first
'\0'. which i think is 2.

> That's it, I'm breaking the 6 we've already got out into their own
> section in the help text...

yeah, however you think the others should be order, this subset is
crazy enough to warrant it.

>> the primary use case is that in a bug report we include a top, and the
>> last two columns will be the thread name and process name. i guess the
>> interesting question is "what should interactive top -H show?". looks
>> like desktop top uses thread name, so if you change nothing, you'll be
>> consistent with existing expectations :-)
>
> *blink* *blink*
>
> I just tried Ubuntu's top and scrolled right because in mine that
> changes the sort fields, and instead it scrolled the display to the side
> but didn't make the output longer.
>
> Right. (And they're highlighting the "R" processes. I wonder if I should
> do that?)
>
> Grrr. Their "top -H" takes the same cpu as my "top", but MY top -H is
> taking 10% of the cpu on my netbook. And it's all the data collection
> phase, not display. (At least now that I did buffered versions of
> getpwuid and getgrgid anyway.) I can tell because cursor
> left/right/left/right doesn't hugely increase the CPU usage even though
> I'm doing 5 redisplays per refresh.
>
> (What would be NICE here is if %cpu could split into separate system and
> user percentages. I haven't got display fields for those. Would it be
> cheating to add them to profile top itself? Later, maybe... :)
>
> Anyway, yes, it's showing Chrome_IOTh+ as one of the persistent cpu hogs
> on my system, and that's not an executable name.
>
> Right, lemme wait for you to answer my list question in the previous
> reply hunk before trying to figure out what "correct behavior" looks
> like in this complicated specless program suite. :P
>
>>>>>>> even though you hate them, this is one of the nice things about long
>>>>>>> options. they're easier to remember, and no one cares that you've
>>>>>>> already taken --list-fields because they're not likely to want
>>>>>>> --list-fields to mean anything else.
>>>>>>
>>>>>> I just like there to be a short option corresponding to each long option.
>>>>>
>>>>> many long options just aren't worth a short option.
>>>
>>> If they aren't worth a short option, are they worth _having_?
>>
>> i don't use a toilet plunger often, but when i do i'm usually pretty
>> glad it was there :-)
>
> I agree --bare-longopts and toilet plungers are analogous, yes. Neither
> is really my first choice of user interface for a given problem, and
> avoiding the need for them is probably worth a little effort.
>
> Sometimes they're unavoidable, and I _did_ make infrastructure in
> lib/args.c to handle them...
>
> Rob



-- 
Elliott Hughes - http://who/enh - http://jessies.org/~enh/
Android native code/tools questions? Mail me/drop by/add me as a reviewer.