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.

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

Add omrthread_get_self_thread_time() #7491

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all 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
14 changes: 14 additions & 0 deletions include_core/thread_api.h
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,11 @@ typedef struct omrthread_monitor_walk_state_t {
BOOLEAN lockTaken;
} omrthread_monitor_walk_state_t;

typedef struct omrthread_thread_time_t {
thallium marked this conversation as resolved.
Show resolved Hide resolved
int64_t userTime;
int64_t sysTime;
} omrthread_thread_time_t;

/* ---------------- omrthreadinspect.c ---------------- */

/**
Expand Down Expand Up @@ -1424,6 +1429,15 @@ omrthread_monitor_pin(omrthread_monitor_t monitor, omrthread_t self);
void
omrthread_monitor_unpin(omrthread_monitor_t monitor, omrthread_t self);

/**
* Gets the system and user CPU time of the current thread.
*
* @param[out] threadTime the pointer to the thread time structure
* @return 0 on success or -1 on failure
*/
intptr_t
omrthread_get_self_thread_time(omrthread_thread_time_t *threadTime);

thallium marked this conversation as resolved.
Show resolved Hide resolved
/* forward struct definition */
struct J9ThreadLibrary;

Expand Down
37 changes: 37 additions & 0 deletions thread/common/thrprof.c
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,11 @@
* APIs for querying per-thread statistics: CPU usage, stack usage.
*/

#if defined (LINUX)
#define _GNU_SOURCE
#include <sys/resource.h>
#endif /* defined (LINUX) */
thallium marked this conversation as resolved.
Show resolved Hide resolved
thallium marked this conversation as resolved.
Show resolved Hide resolved

#include <string.h> /* for memset() */
#include "omrcfg.h"

Expand Down Expand Up @@ -1025,3 +1030,35 @@ omrthread_get_jvm_cpu_usage_info_error_recovery(void)
GLOBAL_UNLOCK_SIMPLE(lib);
}
}

intptr_t
omrthread_get_self_thread_time(omrthread_thread_time_t *threadTime)
thallium marked this conversation as resolved.
Show resolved Hide resolved
{
#if defined(LINUX)
struct rusage rUsage = {0};
thallium marked this conversation as resolved.
Show resolved Hide resolved
int rc = getrusage(RUSAGE_THREAD, &rUsage);

if (-1 == rc) {
return -1;
}

threadTime->userTime = (SEC_TO_NANO_CONVERSION_CONSTANT * (int64_t)rUsage.ru_utime.tv_sec)
+ (MICRO_TO_NANO_CONVERSION_CONSTANT * (int64_t)rUsage.ru_utime.tv_usec);
threadTime->sysTime = (SEC_TO_NANO_CONVERSION_CONSTANT * (int64_t)rUsage.ru_stime.tv_sec)
+ (MICRO_TO_NANO_CONVERSION_CONSTANT * (int64_t)rUsage.ru_stime.tv_usec);
#else /* defined(LINUX) */
omrthread_t self = omrthread_self();

int64_t userTime = omrthread_get_user_time(self);
int64_t cpuTime = omrthread_get_cpu_time(self);
Comment on lines +1052 to +1053
Copy link
Member

Choose a reason for hiding this comment

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

Given these existing API functions, what's the real value of adding this function?

If we decide to go ahead with this change, this should use omrthread_get_self_cpu_time() and omrthread_get_self_user_time() instead.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I see what you mean. Yeah, I think the best way is to add Linux implementation to omrthread_get_self_user_time() so we don't need to add a new function. Then we'll be calling omrthread_get_self_cpu_time() and omrthread_get_self_user_time() on the J9 side. @tajila what do you think?

Copy link
Contributor

Choose a reason for hiding this comment

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

I was hoping to avoid calling getrusage twice, since we need both cpu and user time. What does the support look like on other platforms, are they separate system calls or can those be bundled as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Looks like on Windows and AIX they can be bundled as well.

Copy link
Member

Choose a reason for hiding this comment

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

If we're concerned about making two system calls where one would suffice, I suggest this function should be used by omrthread_get_self_cpu_time() and omrthread_get_self_user_time() to reduce code duplication.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is not a good idea because on some platforms omrthread_get_self_cpu_time get CPU time directly rather than by adding user time and system time.


if ((-1 == cpuTime) || (-1 == userTime)) {
return -1;
}

threadTime->sysTime = cpuTime - userTime;
threadTime->userTime = userTime;
#endif /* defined(LINUX) */

return 0;
}