Skip to content
Open
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions darwin/DarwinProcess.c
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,8 @@ const ProcessFieldData Process_fields[LAST_PROCESSFIELD] = {
[PROC_EXE] = { .name = "EXE", .title = "EXE ", .description = "Basename of exe of the process from /proc/[pid]/exe", .flags = 0, },
[CWD] = { .name = "CWD", .title = "CWD ", .description = "The current working directory of the process", .flags = PROCESS_FLAG_CWD, },
[TRANSLATED] = { .name = "TRANSLATED", .title = "T ", .description = "Translation info (T translated, N native)", .flags = 0, },
[TIME_GPU] = { .name = "GPU_TIME", .title = "GPU TIME", .description = "Total GPU time", .flags = PROCESS_FLAG_GPU, .defaultSortDesc = true, },
[PERCENT_GPU] = { .name = "GPU_PERCENT", .title = " GPU% ", .description = "Percentage of the GPU time the process used in the last sampling", .flags = PROCESS_FLAG_GPU, .defaultSortDesc = true, },
};

Process* DarwinProcess_new(const Machine* host) {
Expand All @@ -77,14 +79,18 @@ void Process_delete(Object* cast) {

static void DarwinProcess_rowWriteField(const Row* super, RichString* str, ProcessField field) {
const DarwinProcess* dp = (const DarwinProcess*) super;
const Machine* host = (const Machine*) super->host;

bool coloring = host->settings->highlightMegabytes;
char buffer[256]; buffer[255] = '\0';
int attr = CRT_colors[DEFAULT_COLOR];
size_t n = sizeof(buffer) - 1;

switch (field) {
// add Platform-specific fields here
case TRANSLATED: xSnprintf(buffer, n, "%c ", dp->translated ? 'T' : 'N'); break;
case PERCENT_GPU: Row_printPercentage(dp->gpu_percent, buffer, n, 5, &attr); break;
case TIME_GPU: Row_printNanoseconds(str, dp->gpu_time, coloring); return;
default:
Process_writeField(&dp->super, str, field);
return;
Expand All @@ -101,6 +107,15 @@ static int DarwinProcess_compareByKey(const Process* v1, const Process* v2, Proc
// add Platform-specific fields here
case TRANSLATED:
return SPACESHIP_NUMBER(p1->translated, p2->translated);
case PERCENT_GPU: {
int r = compareRealNumbers(p1->gpu_percent, p2->gpu_percent);
if (r)
return r;

return SPACESHIP_NUMBER(p1->gpu_time, p2->gpu_time);
}
case TIME_GPU:
return SPACESHIP_NUMBER(p1->gpu_time, p2->gpu_time);
default:
return Process_compareByKey_Base(v1, v2, key);
}
Expand Down
8 changes: 8 additions & 0 deletions darwin/DarwinProcess.h
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ in the source distribution for its full text.


#define PROCESS_FLAG_TTY 0x00000100
#define PROCESS_FLAG_GPU 0x00000200

typedef struct DarwinProcess_ {
Process super;
Expand All @@ -22,6 +23,13 @@ typedef struct DarwinProcess_ {
uint64_t stime;
bool taskAccess;
bool translated;

/* Total GPU time used in nano seconds */
uint64_t gpu_time;
/* GPU utilization in percent */
float gpu_percent;
/* Got GPU time info from last scan */
bool gpu_time_updated;
} DarwinProcess;

extern const ProcessClass DarwinProcess_class;
Expand Down
10 changes: 10 additions & 0 deletions darwin/DarwinProcessTable.c
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,8 @@ void ProcessTable_delete(Object* cast) {
void ProcessTable_goThroughEntries(ProcessTable* super) {
const Machine* host = super->super.host;
const DarwinMachine* dhost = (const DarwinMachine*) host;
const Settings* settings = host->settings;
const ScreenSettings* ss = settings->ss;
DarwinProcessTable* dpt = (DarwinProcessTable*) super;
bool preExisting = true;
struct kinfo_proc* ps;
Expand Down Expand Up @@ -122,12 +124,20 @@ void ProcessTable_goThroughEntries(ProcessTable* super) {
DarwinProcess_scanThreads(proc, dpt);
}

// Reset GPU time updated flag
proc->gpu_time_updated = false;

super->totalTasks += 1;

if (!preExisting) {
ProcessTable_add(super, &proc->super);
}
}

// Get GPU usage info for processes if requested
if (ss->flags & PROCESS_FLAG_GPU) {
Platform_setGPUProcesses(dpt);
}

free(ps);
}
95 changes: 95 additions & 0 deletions darwin/Platform.c
Original file line number Diff line number Diff line change
Expand Up @@ -798,3 +798,98 @@ static void Platform_getOSRelease(char* buffer, size_t bufferLen) {
const char* Platform_getRelease(void) {
return Generic_unameRelease(Platform_getOSRelease);
}

void Platform_setGPUProcesses(DarwinProcessTable* dpt) {
const Machine* host = dpt->super.super.host;
const DarwinMachine* dhost = (const DarwinMachine*) host;

if (!dhost->GPUService)
return;

io_iterator_t iterator;
if (IORegistryEntryGetChildIterator(dhost->GPUService, kIOServicePlane, &iterator) != KERN_SUCCESS)
return;

io_registry_entry_t entry;
while ((entry = IOIteratorNext(iterator))) {
io_struct_inband_t buffer;
uint32_t size = sizeof(buffer);
if (IORegistryEntryGetProperty(entry, "IOUserClientCreator", buffer, &size) != KERN_SUCCESS)
goto cleanup;

pid_t pid;
if (sscanf(buffer, "pid %d", &pid) != 1)
goto cleanup;
Comment on lines +816 to +823

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Please provide some (short) inline documentation about what the contents of buffers are expected to be. Reference to corresponding documentation is fine; throw a copy to archive.org if possible in case the link breaks.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

By the way, I don't like the assumption that pid_t always has the same width as int. It's better to sscanf into an int-type buffer then cast to pid_t when using.


uint64_t gpuTime = 0;
const Table* table = &dpt->super.super;
DarwinProcess* dp = (DarwinProcess*) Hashtable_get(table->table, pid);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Use ProcessTable_findProcess cast to DarwinProcess*, which is always valid on Darwin. Will return NULL if no such process exists.

Also avoids this extra variable only used once.

if (!dp)
goto cleanup;

if (IOObjectConformsTo(entry, "IOGPUDeviceUserClient")) {
CFArrayRef appUsage = IORegistryEntryCreateCFProperty(entry, CFSTR("AppUsage"), kCFAllocatorDefault, kNilOptions);
if (!appUsage)
goto cleanup;

assert(CFGetTypeID(appUsage) == CFArrayGetTypeID());

// Sum up the GPU time for all command queues for this client
size_t count = CFArrayGetCount(appUsage);
for (size_t i = 0; i < count; ++i) {
CFDictionaryRef appUsageEntry = CFArrayGetValueAtIndex(appUsage, i);
assert(CFGetTypeID(appUsageEntry) == CFDictionaryGetTypeID());

CFNumberRef accumulatedGPUTimeRef = CFDictionaryGetValue(appUsageEntry, CFSTR("accumulatedGPUTime"));
if (!accumulatedGPUTimeRef) {
CFRelease(appUsage);
goto cleanup;
}
Comment thread
BenBE marked this conversation as resolved.
Outdated

uint64_t accumulatedGPUTime = 0;
CFNumberGetValue(accumulatedGPUTimeRef, kCFNumberLongLongType, &accumulatedGPUTime);

gpuTime += accumulatedGPUTime;
Comment thread
BenBE marked this conversation as resolved.
}

CFRelease(appUsage);
} else if (IOObjectConformsTo(entry, "IOAccelSubmitter2")) {
CFNumberRef accumulatedGPUTimeRef = IORegistryEntryCreateCFProperty(entry, CFSTR("accumulatedGPUTime"), kCFAllocatorDefault, kNilOptions);
if (!accumulatedGPUTimeRef)
goto cleanup;

uint64_t accumulatedGPUTime = 0;
CFNumberGetValue(accumulatedGPUTimeRef, kCFNumberLongLongType, &accumulatedGPUTime);

gpuTime += accumulatedGPUTime;
Comment thread
BenBE marked this conversation as resolved.

CFRelease(accumulatedGPUTimeRef);
} else {
goto cleanup;
}

if (gpuTime > 0) {
uint64_t gputimeDelta = saturatingSub(gpuTime, dp->gpu_time);
uint64_t monotonicTimeDelta = host->monotonicMs - host->prevMonotonicMs;
dp->gpu_percent = 100.0F * gputimeDelta / (1000 * 1000) / monotonicTimeDelta;
dp->gpu_time = gpuTime;
dp->gpu_time_updated = true;
}

cleanup:
IOObjectRelease(entry);
}

// Clear GPU time and percent for processes not found in the GPU process list
const Vector* rows = dpt->super.super.rows;
int table_size = Vector_size(rows);
for (int i = 0; i < table_size; ++i) {
DarwinProcess* dp = (DarwinProcess*) Vector_get(rows, i);
if (!dp->gpu_time_updated) {
dp->gpu_time = 0;
dp->gpu_percent = 0.0F;
}
}

IOObjectRelease(iterator);
}
2 changes: 2 additions & 0 deletions darwin/Platform.h
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,8 @@ static inline void Platform_getHostname(char* buffer, size_t size) {

const char* Platform_getRelease(void);

void Platform_setGPUProcesses(DarwinProcessTable* dpt);

static inline const char* Platform_getFailedState(void) {
return NULL;
}
Expand Down
2 changes: 2 additions & 0 deletions darwin/ProcessField.h
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ in the source distribution for its full text.

#define PLATFORM_PROCESS_FIELDS \
TRANSLATED = 100, \
TIME_GPU = 101, \
PERCENT_GPU = 102, \
\
DUMMY_BUMP_FIELD = CWD, \
// End of list
Expand Down
12 changes: 6 additions & 6 deletions linux/LinuxProcess.c
Original file line number Diff line number Diff line change
Expand Up @@ -110,8 +110,8 @@ const ProcessFieldData Process_fields[LAST_PROCESSFIELD] = {
#ifdef SCHEDULER_SUPPORT
[SCHEDULERPOLICY] = { .name = "SCHEDULERPOLICY", .title = "SCHED ", .description = "Current scheduling policy of the process", .flags = PROCESS_FLAG_SCHEDPOL, },
#endif
[GPU_TIME] = { .name = "GPU_TIME", .title = "GPU_TIME ", .description = "Total GPU time", .flags = PROCESS_FLAG_LINUX_GPU, .defaultSortDesc = true, },
[GPU_PERCENT] = { .name = "GPU_PERCENT", .title = " GPU% ", .description = "Percentage of the GPU time the process used in the last sampling", .flags = PROCESS_FLAG_LINUX_GPU, .defaultSortDesc = true, },
[TIME_GPU] = { .name = "GPU_TIME", .title = "GPU TIME", .description = "Total GPU time", .flags = PROCESS_FLAG_LINUX_GPU, .defaultSortDesc = true, },

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Given this is meant as an analog to TIME+ for indicating the total, including the + is fine.

[PERCENT_GPU] = { .name = "GPU_PERCENT", .title = " GPU% ", .description = "Percentage of the GPU time the process used in the last sampling", .flags = PROCESS_FLAG_LINUX_GPU, .defaultSortDesc = true, },
};

Process* LinuxProcess_new(const Machine* host) {
Expand Down Expand Up @@ -245,8 +245,8 @@ static void LinuxProcess_rowWriteField(const Row* super, RichString* str, Proces
switch (field) {
case CMINFLT: Row_printCount(str, lp->cminflt, coloring); return;
case CMAJFLT: Row_printCount(str, lp->cmajflt, coloring); return;
case GPU_PERCENT: Row_printPercentage(lp->gpu_percent, buffer, n, 5, &attr); break;
case GPU_TIME: Row_printNanoseconds(str, lp->gpu_time, coloring); return;
case PERCENT_GPU: Row_printPercentage(lp->gpu_percent, buffer, n, 5, &attr); break;
case TIME_GPU: Row_printNanoseconds(str, lp->gpu_time, coloring); return;
case M_DRS: Row_printBytes(str, lp->m_drs * lhost->pageSize, coloring); return;
case M_LRS:
if (lp->m_lrs) {
Expand Down Expand Up @@ -455,14 +455,14 @@ static int LinuxProcess_compareByKey(const Process* v1, const Process* v2, Proce
return SPACESHIP_NUMBER(p1->autogroup_id, p2->autogroup_id);
case AUTOGROUP_NICE:
return SPACESHIP_NUMBER(p1->autogroup_nice, p2->autogroup_nice);
case GPU_PERCENT: {
case PERCENT_GPU: {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I have a question. What motivated you to rename the field from GPU_PERCENT to PERCENT_GPU? (Similar question to GPU_TIME to TIME_GPU.)

For me, the renaming of the fields doesn't make them easier to read.

This is beside the compatibility problem I pointed out to you before, which you have fixed. I just wonder why this idea in the first place.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Because I noticed that the previous percentages were all named like PERCENT_CPU and PERCENT_MEM. Of course, due to compatibility issues, these modifications are now meaningless. Should I revert it back to GPU_PERCENT?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Because I noticed that the previous percentages were all named like PERCENT_CPU and PERCENT_MEM.

Oh I see. Thus it was an error when the GPU percentage fields were introduced in htop Linux monitoring.

Because htop had no requirement on whether the word PERCENT should be placed at the start or at the end.

If you grep -r TIME in the htop codebase, you would get what I mean: We had a lot of fields with "TIME" word at the end. STARTTIME, CPU_TOTAL_TIME, GUEST_TIME, etc.

Of course, due to compatibility issues, these modifications are now meaningless. Should I revert it back to GPU_PERCENT?

It's not my call. Maybe BenBE can decide this.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Given there are other fields like PERCENT_CPU etc. I actually prefer this change. Otherwise I'd already mention this change. Thus can stay as is rn. ;-)

int r = compareRealNumbers(p1->gpu_percent, p2->gpu_percent);
if (r)
return r;

return SPACESHIP_NUMBER(p1->gpu_time, p2->gpu_time);
}
case GPU_TIME:
case TIME_GPU:
return SPACESHIP_NUMBER(p1->gpu_time, p2->gpu_time);
case ISCONTAINER:
return SPACESHIP_NUMBER(v1->isRunningInContainer, v2->isRunningInContainer);
Expand Down