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

Context ID functionality #576

Closed
wants to merge 18 commits into from
Closed

Context ID functionality #576

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
f955463
ecid functionality - wip
krzysztofslusarski Apr 3, 2022
8b78baf
Update profiler.h
krzysztofslusarski Apr 3, 2022
ca82a85
Move ecid to ExecutionEvent for now...
krzysztofslusarski Apr 3, 2022
15b11b1
ECID for other modes
krzysztofslusarski Apr 3, 2022
92cd7db
Merge branch 'master' into ecid
krzysztofslusarski Apr 3, 2022
051280a
ECID for other modes
krzysztofslusarski Apr 3, 2022
34d2327
Controll of tls model
krzysztofslusarski Apr 9, 2022
1be3492
Swap to pthread_set/get
krzysztofslusarski Apr 12, 2022
743959c
Swap to pthread_set/get
krzysztofslusarski Apr 12, 2022
ef6eda2
Changes after PR comments
krzysztofslusarski Apr 19, 2022
7df3852
single malloc
krzysztofslusarski Apr 20, 2022
ac5a185
PR comments
krzysztofslusarski May 5, 2022
60bbb31
PR comments
krzysztofslusarski May 5, 2022
f8cf479
PR comments
krzysztofslusarski May 5, 2022
5f57bc5
Merge remote-tracking branch 'origin/master' into ecid
krzysztofslusarski May 5, 2022
bc0500f
PR comments
krzysztofslusarski May 5, 2022
aa42efd
Javadoc
krzysztofslusarski May 6, 2022
074a6cc
Merge branch 'master' into ecid
Feb 13, 2023
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
19 changes: 19 additions & 0 deletions src/api/one/profiler/AsyncProfiler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -276,11 +276,30 @@ private void filterThread(Thread thread, boolean enable) {
}
}

/**
* Passing context identifier to a profiler. This ID is thread-local and is dumped in
* the JFR output only. 0 is a reserved value for "no-context". The context functionality
* is available for 64bit Java only.
*
* @param contextId Context identifier that should be stored for current thread
*/
public void setContextId(long contextId) {
Copy link

Choose a reason for hiding this comment

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

Can you add javadoc here explaining that 0 value is reserved for 'no-context' and as such should not be used to refer to a valid context?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

setContextId0(contextId);
}

/**
* Clears context identifier for current thread.
*/
public void clearContextId() {
setContextId0(0);
}

private native void start0(String event, long interval, boolean reset) throws IllegalStateException;

private native void stop0() throws IllegalStateException;

private native String execute0(String command) throws IllegalArgumentException, IllegalStateException, IOException;

private native void filterThread0(Thread thread, boolean enable);
private native void setContextId0(long contextId);
}
4 changes: 4 additions & 0 deletions src/flightRecorder.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1114,6 +1114,7 @@ class Recording {
buf->putVar32(tid);
buf->putVar32(call_trace_id);
buf->putVar32(event->_thread_state);
buf->putVar64(Profiler::instance()->getContextId());
Copy link

Choose a reason for hiding this comment

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

Adding an extra event attribute while overloading the JDK defined events will make the JMC parser quite unhappy. It will deal with it but for the price of doubling the memory required for the event metadata - basically, it will have to keep the JDK version as well as async-profiler version.

In addition to the memory overhead there will be also CPU overhead because the parser will need to do the translation for each single event - an initial attempt to parse the event according to the JDK metadata will fail so the alternative metadata will have to be searched and used.

A remedy for this would be using custom event IDs - the labels and even the names can stay the same. The only important thing is that the IDs are not clashing.

I apologise for complicating the matters here but I want to avoid any nasty surprises when users are trying to consume JFRs produced by async-profiler.

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 apologise for complicating the matters

Nothing to apologies for, every comment that makes opensource software better is a good one.

The other option would be to enable whole contextId feature by a flag that is disabled by default. If someone use contextId then there is a high chance that he/she is going to use different JFR viewer since JMC doesn't support contextId.

@apangin any thoughts on that?

Copy link

Choose a reason for hiding this comment

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

If someone use contextId then there is a high chance that he/she is going to use different JFR viewer since JMC doesn't support contextId

contextId is written as a standard event attribute. Since JFR format is self-describing you will be able to see the contextId attribute value for events in JMC.

I would really strongly suggest using custom event IDs (above 10000 eg. so we have a plenty of headroom for any builtin native events as well as any user defined Java events) - it would make it less of a hack. Let's face it - the events are going to have a different structure now so it is just fair that they are distinguishable from the builtin ones.

And, as I already mentioned - we still can keep all the metadata so eg. the execution samples are picked by JMC to present the profiling view etc.

Copy link

Choose a reason for hiding this comment

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

Looking forward to this feature will be released!

Regarding this discussion, I realized that converter's JfrReader would not work if context id is introduced, because though the JFR format itself is self-describing, JfrReader hard codes the parsing logic
https://github.com/async-profiler/async-profiler/blob/v2.9/src/converter/one/jfr/JfrReader.java#L155-L158

Should we fix JfrReader as well?

buf->put8(start, buf->offset() - start);
}

Expand All @@ -1126,6 +1127,7 @@ class Recording {
buf->putVar32(event->_class_id);
buf->putVar64(event->_instance_size);
buf->putVar64(event->_total_size);
buf->putVar64(Profiler::instance()->getContextId());
buf->put8(start, buf->offset() - start);
}

Expand All @@ -1137,6 +1139,7 @@ class Recording {
buf->putVar32(call_trace_id);
buf->putVar32(event->_class_id);
buf->putVar64(event->_total_size);
buf->putVar64(Profiler::instance()->getContextId());
buf->put8(start, buf->offset() - start);
}

Expand All @@ -1162,6 +1165,7 @@ class Recording {
buf->putVar32(event->_class_id);
buf->put8(0);
buf->putVar64(event->_address);
buf->putVar64(Profiler::instance()->getContextId());
buf->put8(start, buf->offset() - start);
}

Expand Down
20 changes: 15 additions & 5 deletions src/javaApi.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,15 @@ Java_one_profiler_AsyncProfiler_stop0(JNIEnv* env, jobject unused) {
}
}

extern "C" DLLEXPORT void JNICALL
Java_one_profiler_AsyncProfiler_setContextId0(JNIEnv* env, jobject unused, jlong contextId) {
Error error = Profiler::instance()->setContextId(contextId);

if (error) {
throwNew(env, "java/lang/IllegalStateException", error.message());
}
}

extern "C" DLLEXPORT jstring JNICALL
Java_one_profiler_AsyncProfiler_execute0(JNIEnv* env, jobject unused, jstring command) {
Arguments args;
Expand Down Expand Up @@ -133,11 +142,12 @@ Java_one_profiler_AsyncProfiler_filterThread0(JNIEnv* env, jobject unused, jthre
#define F(name, sig) {(char*)#name, (char*)sig, (void*)Java_one_profiler_AsyncProfiler_##name}

static const JNINativeMethod profiler_natives[] = {
F(start0, "(Ljava/lang/String;JZ)V"),
F(stop0, "()V"),
F(execute0, "(Ljava/lang/String;)Ljava/lang/String;"),
F(getSamples, "()J"),
F(filterThread0, "(Ljava/lang/Thread;Z)V"),
F(start0, "(Ljava/lang/String;JZ)V"),
F(stop0, "()V"),
F(execute0, "(Ljava/lang/String;)Ljava/lang/String;"),
F(getSamples, "()J"),
F(filterThread0, "(Ljava/lang/Thread;Z)V"),
F(setContextId0, "(J)V"),
};

static const JNINativeMethod* execute0 = &profiler_natives[2];
Expand Down
12 changes: 8 additions & 4 deletions src/jfrMetadata.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,8 @@ JfrMetadata::JfrMetadata() : Element("root") {
<< field("startTime", T_LONG, "Start Time", F_TIME_TICKS)
<< field("sampledThread", T_THREAD, "Thread", F_CPOOL)
<< field("stackTrace", T_STACK_TRACE, "Stack Trace", F_CPOOL)
<< field("state", T_THREAD_STATE, "Thread State", F_CPOOL))
<< field("state", T_THREAD_STATE, "Thread State", F_CPOOL)
<< field("contextId", T_LONG, "Context ID"))

<< (type("jdk.ObjectAllocationInNewTLAB", T_ALLOC_IN_NEW_TLAB, "Allocation in new TLAB")
<< category("Java Application")
Expand All @@ -99,15 +100,17 @@ JfrMetadata::JfrMetadata() : Element("root") {
<< field("stackTrace", T_STACK_TRACE, "Stack Trace", F_CPOOL)
<< field("objectClass", T_CLASS, "Object Class", F_CPOOL)
<< field("allocationSize", T_LONG, "Allocation Size", F_BYTES)
<< field("tlabSize", T_LONG, "TLAB Size", F_BYTES))
<< field("tlabSize", T_LONG, "TLAB Size", F_BYTES)
<< field("contextId", T_LONG, "Context ID"))

<< (type("jdk.ObjectAllocationOutsideTLAB", T_ALLOC_OUTSIDE_TLAB, "Allocation outside TLAB")
<< category("Java Application")
<< field("startTime", T_LONG, "Start Time", F_TIME_TICKS)
<< field("eventThread", T_THREAD, "Event Thread", F_CPOOL)
<< field("stackTrace", T_STACK_TRACE, "Stack Trace", F_CPOOL)
<< field("objectClass", T_CLASS, "Object Class", F_CPOOL)
<< field("allocationSize", T_LONG, "Allocation Size", F_BYTES))
<< field("allocationSize", T_LONG, "Allocation Size", F_BYTES)
<< field("contextId", T_LONG, "Context ID"))

<< (type("jdk.JavaMonitorEnter", T_MONITOR_ENTER, "Java Monitor Blocked")
<< category("Java Application")
Expand All @@ -117,7 +120,8 @@ JfrMetadata::JfrMetadata() : Element("root") {
<< field("stackTrace", T_STACK_TRACE, "Stack Trace", F_CPOOL)
<< field("monitorClass", T_CLASS, "Monitor Class", F_CPOOL)
<< field("previousOwner", T_THREAD, "Previous Monitor Owner", F_CPOOL)
<< field("address", T_LONG, "Monitor Address", F_ADDRESS))
<< field("address", T_LONG, "Monitor Address", F_ADDRESS)
<< field("contextId", T_LONG, "Context ID"))

<< (type("jdk.ThreadPark", T_THREAD_PARK, "Java Thread Park")
<< category("Java Application")
Expand Down
20 changes: 20 additions & 0 deletions src/profiler.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <pthread.h>
#include <sys/param.h>
#include "profiler.h"
#include "perfEvents.h"
Expand All @@ -45,6 +46,7 @@
#include "symbols.h"
#include "vmStructs.h"

void __attribute__((constructor)) createContextId();

// The instance is not deleted on purpose, since profiler structures
// can be still accessed concurrently during VM termination
Expand All @@ -64,6 +66,11 @@ static J9WallClock j9_wall_clock;
static ITimer itimer;
static Instrument instrument;

static pthread_key_t local_context_id_key;

void createContextId() {
pthread_key_create(&local_context_id_key, NULL);
}

// Stack recovery techniques used to workaround AsyncGetCallTrace flaws.
// Can be disabled with 'safemode' option.
Expand Down Expand Up @@ -1080,6 +1087,19 @@ Error Profiler::start(Arguments& args, bool reset) {
return error;
}

Error Profiler::setContextId(u64 contextId) {
if (sizeof(uintptr_t) < sizeof(u64)) {
return Error("Setting contextId is supported on 64bit Java only");
}
pthread_setspecific(local_context_id_key, (void *) contextId);
return Error::OK;
}

u64 Profiler::getContextId() {
void* value = pthread_getspecific(local_context_id_key);
return (u64) value;
}

Error Profiler::stop() {
MutexLocker ml(_state_lock);
if (_state != RUNNING) {
Expand Down
3 changes: 3 additions & 0 deletions src/profiler.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ class Profiler {
// TODO: single map?
std::map<int, std::string> _thread_names;
std::map<int, jlong> _thread_ids;

Dictionary _class_map;
Dictionary _symbol_map;
ThreadFilter _thread_filter;
Expand Down Expand Up @@ -198,6 +199,8 @@ class Profiler {
void shutdown(Arguments& args);
Error check(Arguments& args);
Error start(Arguments& args, bool reset);
Error setContextId(u64 contextId);
u64 getContextId();
Error stop();
Error flushJfr();
Error dump(std::ostream& out, Arguments& args);
Expand Down