-
Notifications
You must be signed in to change notification settings - Fork 0
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
Clean enable lttng commits #16
base: tracing
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Классно, работает dtrace вместе с lttng!
Комментов много, но много и пересекаются. Кажется, по несколько можно фиксить одним изменением. Лучше сначала прочитать всё
Пробую другие комбинации dtrace/lttng.
Полезно иметь несколько конфигов сборок в одной ждк
configure --with-conf-name=dt-lt --enable-dtrace=yes --enable-lttng=yes
make CONF=dt-lt images
Сборка будет в build/dt-lt/...
dev-tracing/h_generator/run.sh
Outdated
java Main hotspot.d $1/src/hotspot/share/utilities/hotspotLTTngDtrace.h lttng dtrace | ||
java Main hotspot.d $1/src/hotspot/share/utilities/hotspotDtrace.h dtrace | ||
java Main hs_private.d $1/src/hotspot/share/utilities/hs_privateLTTngDtrace.h lttng dtrace | ||
java Main hs_private.d $1/src/hotspot/share/utilities/hs_privateDtrace.h dtrace |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hs_private.d
и hotspot.d
скопированы. А можно прописать относительные пути до них, ../../src/hotspot/os/posix/dtrace/hotspot.d
, а копии удалить?
@@ -50,6 +51,14 @@ | |||
#include "dtracefiles/hotspot_jni.h" | |||
#include "dtracefiles/hs_private.h" | |||
|
|||
#elif defined(LTTNG_ENABLED) /* defined(DTRACE_ENABLED) */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Это только для объявления DTRACE_ONLY
? Можно ли объединить с предыдущим блоком?
#if defined(DTRACE_ENABLED) || defined(LTTNG_ENABLED)
#define DTRACE_ONLY(x) x
...
#if defined(DTRACE_ENABLED)
... // специфика для DTRACE
#endif // DTRACE_ENABLED
#endif // DTRACE_ENABLED || LTTNG_ENABLED
@@ -0,0 +1,126 @@ | |||
#define HOTSPOT_CLASS_LOADED_WRAPPER(arg0,arg1,arg2,arg3) \ | |||
HOTSPOT_CLASS_LOADED(arg0,arg1,arg2,arg3)tracepoint(hotspot, class__loaded, arg0,arg1,arg2,arg3) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Можно ли совместить hotspotDtrace.h и hotspotLTTngDtrace.h?
hotspotLTTngDtrace.h отличаются только #include "utilities/lttng.hpp"
и ;
между dtrace probe и lttng tracpoint
#include "utilities/lttng.hpp"
#define HOTSPOT_CLASS_LOADED_WRAPPER(arg0,arg1,arg2,arg3) \
HOTSPOT_CLASS_LOADED(arg0,arg1,arg2,arg3);tracepoint(hotspot, class__loaded, arg0,arg1,arg2,arg3)
#define HOTSPOT_CLASS_UNLOADED_WRAPPER(arg0,arg1,arg2,arg3) \
HOTSPOT_CLASS_UNLOADED(arg0,arg1,arg2,arg3);tracepoint(hotspot, class__unloaded, arg0,arg1,arg2,arg3)
Можно было бы
#ifdef LTTNG_ENABLED
#include "utilities/lttng.hpp"
#else
#define tracepoint(...)
#endif
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Сломался билд с одним только dtrace'ом:
- из-за отсутствия
;
- из-за отсутствия декларации tracepoint
Вот такое чинит билд
diff --git a/src/hotspot/share/utilities/hotspotDtrace.h b/src/hotspot/share/utilities/hotspotDtrace.h
index f2487e26f4..5793217e40 100644
--- a/src/hotspot/share/utilities/hotspotDtrace.h
+++ b/src/hotspot/share/utilities/hotspotDtrace.h
@@ -1,126 +1,128 @@
+#define tracepoint(...)
+
#define HOTSPOT_CLASS_LOADED_WRAPPER(arg0,arg1,arg2,arg3) \
-HOTSPOT_CLASS_LOADED(arg0,arg1,arg2,arg3)tracepoint(hotspot, class__loaded, arg0,arg1,arg2,arg3)
+HOTSPOT_CLASS_LOADED(arg0,arg1,arg2,arg3);tracepoint(hotspot, class__loaded, arg0,arg1,arg2,arg3)
#define HOTSPOT_CLASS_UNLOADED_WRAPPER(arg0,arg1,arg2,arg3) \
-HOTSPOT_CLASS_UNLOADED(arg0,arg1,arg2,arg3)tracepoint(hotspot, class__unloaded, arg0,arg1,arg2,arg3)
+HOTSPOT_CLASS_UNLOADED(arg0,arg1,arg2,arg3);tracepoint(hotspot, class__unloaded, arg0,arg1,arg2,arg3)
@@ -0,0 +1,128 @@ | |||
#include "utilities/lttng.hpp" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Наверно лучше напрямую включить utilities/lttngTracepoints.hpp
@@ -0,0 +1,20 @@ | |||
#include "utilities/lttng.hpp" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
utilities/lttngHSPrivate.hpp
@@ -0,0 +1,7 @@ | |||
#ifndef SHARE_UTILITIES_LTTNG_HPP |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Этот файл можно удалить. Кажется, он существует, только чтобы генератор включал именно его. Но в итоге в хидеры включается слишком много трейспоинтов.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
никто не инклюдит хидер, удалить
@@ -28,6 +28,7 @@ | |||
|
|||
#if defined(DTRACE_ENABLED) | |||
|
|||
#define SDT_USE_VARIADIC | |||
#include <sys/sdt.h> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Было бы здорово включать #include <lttng/tracepoint.h>
(системный хидер) где-нибудь неподалёку, в этом же файле.
Возможно, было бы проще в одном месте разрулить, что в каком порядке включать. Например только lttng если DTRACE_ENABLED и LTTNG_UST_HAVE_SDT_INTEGRATION
Если да, то надо удалить #include <lttng/tracepoint.h> из utilities/lttngTracepoints.hpp и utilities/lttngHSPrivate.hpp
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Здорово, работает как надо!
Предлагаю доделывать работу в этом PR/бранче (не мержить). Когда будет всё готово, надо будет ещё раз проревьюить тщательно и подчистить что надо
@@ -0,0 +1,4 @@ | |||
export CLASSPATH=".:/usr/local/lib/antlr-4.8-complete.jar:$CLASSPATH" | |||
java -jar /usr/local/lib/antlr-4.8-complete.jar Probes.g4 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probes.g4 не закоммичен
|
||
#include <utilities/lttngTracepoints.hpp> | ||
#include <utilities/lttngHSPrivate.hpp> | ||
#include <utilities/lttng_hs_jni.hpp> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
не закоммичен utilities/lttng_hs_jni.hpp
dev-tracing/h_generator/Main.java
Outdated
if (isLTTngEnabled && !printNames) { | ||
out.println("#include \"utilities/lttng.hpp\"\n"); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Можно удалить?
dev-tracing/h_generator/Main.java
Outdated
if (dtrace) { | ||
stream.print(probeMacroName + "(" + args + ")"); | ||
} | ||
if (dtrace && lttng) { | ||
stream.print(";"); | ||
} | ||
if (dtrace) { | ||
stream.print("tracepoint" + params); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
всё безусловно?
make/hotspot/gensrc/GensrcLTTng.gmk
Outdated
LTTNG_SOURCE_DIR := $(TOPDIR)/src/hotspot/share/utilities | ||
LTTNG_GENSRC_DIR := $(JVM_VARIANT_OUTPUTDIR)/gensrc/lttng | ||
|
||
$(LTTNG_GENSRC_DIR)/%.o: $(LTTNG_SOURCE_DIR)/%.cpp | ||
$(call MakeDir, $(@D)) | ||
$(CPP) -I$(LTTNG_SOURCE_DIR) -fpic -c $^ -o $@ | ||
|
||
$(LTTNG_GENSRC_DIR)/lib%.so: $(LTTNG_GENSRC_DIR)/%.o | ||
$(CPP) -shared -o $@ $^ -llttng-ust -ldl | ||
|
||
LTTNG_LIB_FILES := $(addprefix $(LTTNG_GENSRC_DIR)/, \ | ||
liblttng_hs_jni.so \ | ||
liblttng_hotspot.so \ | ||
liblttng_hs_private.so \ | ||
) | ||
|
||
TARGETS += $(LTTNG_LIB_FILES) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
JDK uses library make function to define compilation. It's worth use it as it would handle various interesting cases, such as crosscompilation. Example is make/hotspot/lib/CompileDtraceLibraries.gmk
$(eval $(call SetupNativeCompilation, BUILD_LIBJVM_DTRACE, \
NAME := jvm_dtrace, \
OUTPUT_DIR := $(JVM_LIB_OUTPUTDIR), \
SRC := $(TOPDIR)/src/java.base/solaris/native/libjvm_dtrace, \
CFLAGS := $(JNI_INCLUDE_FLAGS) -m64 -G -mt -KPIC -xldscope=hidden, \
LDFLAGS := -m64 -mt -xnolib $(SHARED_LIBRARY_FLAGS), \
LIBS := $(LIBDL) -lthread -ldoor, \
OBJECT_DIR := $(LIBJVM_DTRACE_OUTPUTDIR)/objs, \
DEFINE_THIS_FILE := false, \
))
Not all parameters are needed for our libs, of course.
gcc -o main main.o -ldl | ||
|
||
# LD_PRELOAD=./libtpp.so ./main | ||
./main |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great! I see this starts and generates events on my system with lttng and starts in docker container that doesn't have lttng. That what we need.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would suggest add the dlopen to src/hotspot/os/linux/os_linux.cpp, os::init_2
or nearby
Без lttng не работает. Пока не поняла, в чем пробелема |
Вижу с SetupNativeCompilation получилось, здорово!
Вот так?
libjvm слинкована с lttng-ust. Похоже, только liblttng.so надо линковать с lttng-ust |
Без статической линковки lttng события не пробрасываются. dlopen не видит библиотеку (сейчас кладется вместе с другими библиотеками, но если класть вместе с libjvm, то так же). Если ее добавить в LD_PRELOAD, то событий все равно нет. |
Здесь нет ничего про lttng. А вот если сделать
и перекомпилировать, то
И эвенты начинают приходить. Но в целом-то всё работает! Класс! Я тогда планирую плотно посмотреть на патч. Больше же изменений не предвидится? |
Функциональных изменений не планировалось. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Да вообще всё отлично, совсем минорные комментарии. Будет время исправить?
make/autoconf/libraries.m4
Outdated
if test "x$INCLUDE_LTTNG" = xtrue; then | ||
BASIC_JVM_LIBS="$BASIC_JVM_LIBS -ldl" | ||
fi | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
МБ не надо с тех пор, как компилим трейспоинты в отдельную .so
src/hotspot/os/linux/os_linux.cpp
Outdated
@@ -5201,6 +5201,10 @@ jint os::init_2(void) { | |||
set_coredump_filter(FILE_BACKED_SHARED_BIT); | |||
} | |||
|
|||
#if defined(LTTNG_ENABLED) | |||
dlopen("liblttng.so", RTLD_NOW | RTLD_GLOBAL); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Хочется показать, что мы подумали про ошибку
if (!dlopen()) {
/* a problem with loading lttng.so, lttng-ust.so probably unavilable, ignore */
}
probe ToReflectedMethod__return(void*); | ||
probe UnregisterNatives__entry(void*, void*); | ||
probe UnregisterNatives__return(uint32_t); | ||
probe AllocObject__entry(void* env, void* clazz); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Отступ здесь съехал, нужно добавит хоть руками
@@ -0,0 +1,749 @@ | |||
#if !defined(TRACEPOINT_HEADER_MULTI_READ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Для новых файлов-исходников нужна лицензия, здесь я подумаю
@@ -0,0 +1,7 @@ | |||
#ifndef SHARE_UTILITIES_LTTNG_HPP |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
никто не инклюдит хидер, удалить
@@ -1090,8 +1090,8 @@ | |||
#define HOTSPOT_JNI_GETMODULE_RETURN(arg0) | |||
#define HOTSPOT_JNI_GETMODULE_RETURN_ENABLED() | |||
|
|||
#else /* !defined(DTRACE_ENABLED) */ | |||
#error This file should only be included when dtrace is not enabled | |||
#if defined(DTRACE_ENABLED) && !(defined(LTTNG_ENABLED) && defined(LTTNG_UST_HAVE_SDT_INTEGRATION)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Двинуть вверх на место закомментированного условия
#define TRACEPOINT_CREATE_PROBES | ||
|
||
#include <lttng_hotspot.hpp> | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Не хотим объединить такие 3 файла в один, как в хотспоте
No description provided.