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

Thread Names not updating with collapsed output #511

Open
SakaeKac opened this issue Nov 26, 2021 · 7 comments
Open

Thread Names not updating with collapsed output #511

SakaeKac opened this issue Nov 26, 2021 · 7 comments

Comments

@SakaeKac
Copy link

I have an Java application that uses an Executor to parallelize work. Before each chunk of work, the Runnable renames the worker thread based on the task that it is accomplishing and then resets the name at the end of its work. When I use async-profiler externally, it looks like it connects via MXBeans and takes thread dumps, which end up having the renamed thread names. This is very useful as I often want to focus in on one specific task. But when I run the profiler as an agent using the command and dumping collapsed output

start,event=itimer,threads

The thread names all seem to be the original ThreadFactory-assigned thread names. I tried to look through the code and I think it's caching the thread names based on the OS thread id, so it's only getting the first name that it sees and then that's sticky forever after. I'm wondering if there is any sort of flag, configuration or other method of dumping that would preserve the JVM thread name at the point that the sample is taken and work when running as an agent.

@apangin
Copy link
Collaborator

apangin commented Nov 27, 2021

One Java thread has one name in async-profiler reports.
The profiler takes care of thread renaming, but keeps only the last name.

@SakaeKac
Copy link
Author

Is there any way to get the thread name assigned to the sample when the sample is taken rather than when the profiling is completed?

After reading a bit about this: #106

It looks like JFR was introduced to be able to keep the timestamps of individual samples. If I switch JFR will it also maintain the thread name from the point of the sample? Or is does it just maintain a reference to the thread and I will get whatever name happens to exist at the time that I collect the sample?

@apangin
Copy link
Collaborator

apangin commented Nov 29, 2021

In JFR format, thread names are obtained in the same way.

@SakaeKac
Copy link
Author

Any ideas or hints on where to adjust things to try to maintain the thread name that existed at the point of sample? I believe I understand that it's impossible to eliminate races and it's clear that it would add to the overhead in terms of CPU and memory, but wondering what part of the code would need to be adjusted to be able to even support this?

@apangin
Copy link
Collaborator

apangin commented Dec 3, 2021

Current data model of the profiler implies at most one instance of a given thread (i.e. Java thread can't have more than one identifier).
Changing this would require notable architectural modifications, which are not on my list right now, unfortunately.

@SakaeKac
Copy link
Author

SakaeKac commented Dec 6, 2021

I was looking through the code a bit more to try to figure out where this would happen. It looks like the threadId is pushed onto the stack at

https://github.com/jvm-profiling-tools/async-profiler/blob/170451990ba6b1c16ea54491a90d0ca3d81dee48/src/profiler.cpp#L653-L655

And then read back out at

https://github.com/jvm-profiling-tools/async-profiler/blob/170451990ba6b1c16ea54491a90d0ca3d81dee48/src/frameName.cpp#L245-L257

I understand now that ThreadMap is the map of thread id to the name. So, it's clear that in the profiler you are just grabbing the thread id and then using that to get the name of the thread in the frameName code. The actual updating of the _thread_names appears to happen asynchronously.

It seems like it should be possible to implement something that keeps track of the thread name at time of sample by

  1. Adjusting the code at https://github.com/jvm-profiling-tools/async-profiler/blob/170451990ba6b1c16ea54491a90d0ca3d81dee48/src/profiler.cpp#L653-L655 to lookup the thread name with a jvmti VM::getThreadInfo call, then that name could be pushed into a dictionary.
  2. Adjust the code at https://github.com/jvm-profiling-tools/async-profiler/blob/170451990ba6b1c16ea54491a90d0ca3d81dee48/src/frameName.cpp#L245-L257 to know about the dictionary of names and lookup with that instead. Maybe introduce a new type BCI_THREAD_NAME instead of BCI_THREAD_ID and use that to differentiate?

I'm not well versed in C nor in jvmti so not really sure how easy the above things are. The open questions in my mind are

  1. How expensive is the VM::getThreadInfo call, if that's called for all of the threads at the point of generating the event and it is expensive, this could be a non-starter.
  2. Is there a good object that can be used as a dictionary? I had assumed that there would be something already in the code and looked around, but it looks like the current profiling code always assumes that it can get the strings from the VM or some other part of the system and doesn't actually maintain its own dictionaries. This makes a lot of sense as it minimizes memory usage, but also didn't give me any hints for how easy/hard it is to create a dictionary?

Does this cover the architectural modifications you were talking about? Or is there something I'm missing?

@apangin
Copy link
Collaborator

apangin commented Dec 11, 2021

GetThreadInfo is not safe inside a signal handler.
In general, any function that may lock or allocate, is prohibited within Profiler::recordSample.

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

No branches or pull requests

2 participants