[Toybox] top overhead

[enh]

toybox top's relatively high overhead bugged me again, and a couple of
things stood out from `strace -c`.

one is that we call sysinfo() once for each process. this doesn't take a
_huge_ amount of cpu time, but it's still noticeable. procps and busybox
don't do this, and ironically the code comment says this is _for_ slow
systems:

  // /proc data is generated as it's read, so for maximum accuracy on slow
  // systems (or ps | more) we re-fetch uptime as we fetch each /proc line.
  sysinfo(&si);
  slot[SLOT_uptime] = si.uptime;
  slot[SLOT_totalram] = si.totalram;
  slot[SLOT_upticks] = slot[SLOT_uptime]*TT.ticks - slot[SLOT_starttime];

unfortunately that comment was there since the sysinfo() call was first
introduced, so i'm not sure what the motivating example was there? (and
even if it did matter, would it make more sense to just compute all the
"upticks" slots at the end? uptime and totalram seem like it's _less_
confusing if they're just measured once per snapshot.)

-*-

separately, we're doing a lot more read()s, apparently because we always
read /proc/pid/status in addition to stat and statm. a bit of printf
debugging shows that's because _PS_RUSER is set ... but i'm not sure why?
we're not actually showing that in the default set of fields? i tried to
work out where that was coming from, but it's time to get some fresh air
and i haven't made much progress, so i wanted to at least record/report
that, since this is further than i'd poked before. hacking this out is a
bigger improvement than hacking sysinfo() out.

-*-

even more confusing, i'm struggling to correlate what i'm seeing in strace
with what i expect from the code. given that ps.c is using readfile() i'd
expect an openat(), an fstatat() for the size, and one read(). but i
actually see the openat(), a short read() into a larger buffer, then a
read() that returns 0. oh, because the loop in readfd() is calling
readall(), not read(), and readall() doesn't have the same "short read =>
EOF" that readfd() appears to have [but doesn't actually have, because it's
calling readall() which won't return short reads except on error].

this caution probably isn't necessary for procfs? (procps isn't this
cautious, but busybox looks a lot like toybox under strace, presumably also
calling a generic routine rather than taking advantage of any assumptions
about procfs.)

-*-

but it looks like just fixing whatever's setting _PS_RUSER even though we
don't need it gets us a lot closer to busybox/procps top load. definitely
time for that fresh air though...
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.landley.net/pipermail/toybox-landley.net/attachments/20241116/3b32b919/attachment.htm>