Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Perf #1473

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
+79 −22
Open

Perf #1473

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a45cddb
Annotate mbstowcs_nonfatal() with restrict
cgzones May 6, 2024
7901d3a
Linux: use fast integer parsing for /proc/<pid>/stat
cgzones May 6, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion RichString.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ void RichString_rewind(RichString* this, int count) {

#ifdef HAVE_LIBNCURSESW

static size_t mbstowcs_nonfatal(wchar_t* dest, const char* src, size_t n) {
static size_t mbstowcs_nonfatal(wchar_t* restrict dest, const char* restrict src, size_t n) {
size_t written = 0;
mbstate_t ps = { 0 };
bool broken = false;
Expand Down
99 changes: 78 additions & 21 deletions linux/LinuxProcessTable.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,11 +78,68 @@ static FILE* fopenat(openat_arg_t openatArg, const char* pathname, const char* m
return fp;
}

static long fast_strtol_dec(char** str, int maxlen) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about just calling fast_stroull_dec() and downcast the result for this one?

static long fast_strtol_dec(char** str, int maxlen) {
   long long result = fast_strtoll_dec(str, maxlen);
   assert(result <= LONG_MAX);
   assert(result >= LONG_MIN);
   return (long)result;
}

long result = 0;
bool neg = false;

if (**str == '-') {
neg = true;
(*str)++;
}

if (!maxlen)
maxlen = 20; // −9223372036854775808
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If at all, document the why not the magic value …

And the why is basically "because C does not have default arguments" …


while (maxlen-- && **str >= '0' && **str <= '9') {
result *= 10;
result += **str - '0';
(*str)++;
}

return neg ? -result : result;
}

static unsigned long fast_strtoul_dec(char** str, int maxlen) {
unsigned long result = 0;

if (!maxlen)
maxlen = 20; // 18446744073709551615
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here: Drop this comment …


while (maxlen-- && **str >= '0' && **str <= '9') {
result *= 10;
result += **str - '0';
(*str)++;
}
Comment on lines +108 to +112
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same replacement of this while loop could be done here too …


return result;
}

static long long fast_strtoll_dec(char** str, int maxlen) {
long long result = 0;
bool neg = false;

if (**str == '-') {
neg = true;
(*str)++;
}

if (!maxlen)
maxlen = 20; // −9223372036854775808
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similar here: Drop that comment.


while (maxlen-- && **str >= '0' && **str <= '9') {
result *= 10;
result += **str - '0';
(*str)++;
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This "while" loop can be converted to a fast_strtoull_dec(str, maxlen) and then cast the result from unsigned to signed.


return neg ? -result : result;
}

static inline uint64_t fast_strtoull_dec(char** str, int maxlen) {
register uint64_t result = 0;

if (!maxlen)
--maxlen;
maxlen = 20; // 18446744073709551615
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Documenting the previous default makes no sense for this line. If at all we want documentation it would explain, that this is "assume max reasonable length if none given". I'm for dropping this comment though …

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just FYI, the standard strtoull() permits almost unlimited number of leading zeros in the string. While it won't certainly happen in parsing any Linux procfs file, having an arbitrary length limit in a strtoull() implementation would technically violate ISO C.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The issue here is less about ISO C compliance, but more about squeeze out cycles. fast_strtoull is not meant to comply to ISO C, but to parse the data we actually encounter in realistic situations. Otherwise we'd have to support locales and a full load of other stuff (different radix prefixes) we don't need …


while (maxlen-- && **str >= '0' && **str <= '9') {
result *= 10;
Expand Down Expand Up @@ -306,79 +363,79 @@ static bool LinuxProcessTable_readStatFile(LinuxProcess* lp, openat_arg_t procFd
location += 2;

/* (4) ppid - %d */
Process_setParent(process, strtol(location, &location, 10));
Process_setParent(process, fast_strtol_dec(&location, 0));
location += 1;

/* (5) pgrp - %d */
process->pgrp = strtol(location, &location, 10);
process->pgrp = fast_strtol_dec(&location, 0);
location += 1;

/* (6) session - %d */
process->session = strtol(location, &location, 10);
process->session = fast_strtol_dec(&location, 0);
location += 1;

/* (7) tty_nr - %d */
process->tty_nr = strtoul(location, &location, 10);
process->tty_nr = fast_strtoul_dec(&location, 0);
location += 1;

/* (8) tpgid - %d */
process->tpgid = strtol(location, &location, 10);
process->tpgid = fast_strtol_dec(&location, 0);
location += 1;

/* (9) flags - %u */
lp->flags = strtoul(location, &location, 10);
lp->flags = fast_strtoul_dec(&location, 0);
location += 1;

/* (10) minflt - %lu */
process->minflt = strtoull(location, &location, 10);
process->minflt = fast_strtoull_dec(&location, 0);
location += 1;

/* (11) cminflt - %lu */
lp->cminflt = strtoull(location, &location, 10);
lp->cminflt = fast_strtoull_dec(&location, 0);
location += 1;

/* (12) majflt - %lu */
process->majflt = strtoull(location, &location, 10);
process->majflt = fast_strtoull_dec(&location, 0);
location += 1;

/* (13) cmajflt - %lu */
lp->cmajflt = strtoull(location, &location, 10);
lp->cmajflt = fast_strtoull_dec(&location, 0);
location += 1;

/* (14) utime - %lu */
lp->utime = LinuxProcessTable_adjustTime(lhost, strtoull(location, &location, 10));
lp->utime = LinuxProcessTable_adjustTime(lhost, fast_strtoull_dec(&location, 0));
location += 1;

/* (15) stime - %lu */
lp->stime = LinuxProcessTable_adjustTime(lhost, strtoull(location, &location, 10));
lp->stime = LinuxProcessTable_adjustTime(lhost, fast_strtoull_dec(&location, 0));
location += 1;

/* (16) cutime - %ld */
lp->cutime = LinuxProcessTable_adjustTime(lhost, strtoull(location, &location, 10));
lp->cutime = LinuxProcessTable_adjustTime(lhost, fast_strtoull_dec(&location, 0));
location += 1;

/* (17) cstime - %ld */
lp->cstime = LinuxProcessTable_adjustTime(lhost, strtoull(location, &location, 10));
lp->cstime = LinuxProcessTable_adjustTime(lhost, fast_strtoull_dec(&location, 0));
location += 1;

/* (18) priority - %ld */
process->priority = strtol(location, &location, 10);
process->priority = fast_strtol_dec(&location, 0);
location += 1;

/* (19) nice - %ld */
process->nice = strtol(location, &location, 10);
process->nice = fast_strtol_dec(&location, 0);
location += 1;

/* (20) num_threads - %ld */
process->nlwp = strtol(location, &location, 10);
process->nlwp = fast_strtol_dec(&location, 0);
location += 1;

/* Skip (21) itrealvalue - %ld */
location = strchr(location, ' ') + 1;

/* (22) starttime - %llu */
if (process->starttime_ctime == 0) {
process->starttime_ctime = lhost->boottime + LinuxProcessTable_adjustTime(lhost, strtoll(location, &location, 10)) / 100;
process->starttime_ctime = lhost->boottime + LinuxProcessTable_adjustTime(lhost, fast_strtoll_dec(&location, 0)) / 100;
} else {
location = strchr(location, ' ');
}
Expand All @@ -392,7 +449,7 @@ static bool LinuxProcessTable_readStatFile(LinuxProcess* lp, openat_arg_t procFd
assert(location != NULL);

/* (39) processor - %d */
process->processor = strtol(location, &location, 10);
process->processor = fast_strtol_dec(&location, 0);

/* Ignore further fields */

Expand Down Expand Up @@ -662,7 +719,7 @@ static void LinuxProcessTable_readMaps(LinuxProcess* process, openat_arg_t procF
if (!map_devmaj && !map_devmin)
continue;

map_inode = fast_strtoull_dec(&readptr, 20);
map_inode = fast_strtoull_dec(&readptr, 0);
if (!map_inode)
continue;

Expand Down