Skip to content

Conversation

@nslowell
Copy link

Keeping proc interactions isoltated: added function in proc.h/.c
that prints thread name to stdout. It is then used by backtrace.c
directly to keep from having to store the string in memory in
snapshot or any other struct

Keeping proc interactions isoltated: added function in proc.h/.c
that prints thread name to stdout. It is then used by backtrace.c
directly to keep from having to store the string in memory in
snapshot or any other struct
src/proc.c Outdated
int print_proc_comm(int pid)
{
FILE *f;
char buf[32] = "";
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this initialization really needed?

Copy link
Author

Choose a reason for hiding this comment

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

I believe an earlier implementation required it. It's probably not needed now, however, I feel safer making sure a string is properly terminated.

@nslowell
Copy link
Author

Let me know your thoughts on the new commits. I wasn't able to center the text but I at least squared it up better.

@nslowell
Copy link
Author

Any thoughts on the recent changes?

if (x > 0 && x <= sizeof(end_pad))
{
end_pad[sizeof(end_pad) - x] = '\0';
printf("-------------- thread %d (%d) (%s) %s\n", (index != NULL ? index[i] : i+1), snap->tids[i], comm, end_pad);
Copy link
Contributor

Choose a reason for hiding this comment

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

If get_thread_num() fails for some reason, or the name is too long, the header is not printed at all. This is quite wrong. In case of error it should be indicated in some simple way (like we do when a file is deleted). If the name is too long, we can truncate it.

@v-nikulichev
Copy link
Contributor

@nslowell sorry for missing your updates. I did some review, see my new comment.

src/backtrace.c Outdated
printf("-------------------- thread %d (%d) --------------------\n",
printf("-------------------- thread %d (%d) (",
(index != NULL ? index[i] : i+1), threads[i]);
print_proc_comm(threads[i]);
Copy link

Choose a reason for hiding this comment

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

Is it a good idea to call print_proc_comm here, while the process is stopped?
As print_proc_comm is currently implemented it opens, reads, and closes the comm file for each thread.
In a large server process with thousands of threads, this will add noticeably to the time frozen.
Why not collect the thread names before the call to attach_process(), the same way thread states are handled, then pass the thread name for the current thread to print_thread_heading?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants