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 ITTAPI hooks to jl_mutex_wait #49434

Merged
merged 7 commits into from
Apr 21, 2023
Merged
Show file tree
Hide file tree
Changes from 6 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
4 changes: 2 additions & 2 deletions src/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -157,8 +157,8 @@ LIBJULIA_PATH_REL := libjulia
endif

COMMON_LIBPATHS := -L$(build_libdir) -L$(build_shlibdir)
RT_LIBS := $(WHOLE_ARCHIVE) $(LIBUV) $(WHOLE_ARCHIVE) $(LIBUTF8PROC) $(NO_WHOLE_ARCHIVE) $(LIBUNWIND) $(RT_LLVMLINK) $(OSLIBS) $(LIBTRACYCLIENT)
CG_LIBS := $(LIBUNWIND) $(CG_LLVMLINK) $(OSLIBS) $(LIBTRACYCLIENT)
RT_LIBS := $(WHOLE_ARCHIVE) $(LIBUV) $(WHOLE_ARCHIVE) $(LIBUTF8PROC) $(NO_WHOLE_ARCHIVE) $(LIBUNWIND) $(RT_LLVMLINK) $(OSLIBS) $(LIBTRACYCLIENT) $(LIBITTAPI)
CG_LIBS := $(LIBUNWIND) $(CG_LLVMLINK) $(OSLIBS) $(LIBTRACYCLIENT) $(LIBITTAPI)
RT_DEBUG_LIBS := $(COMMON_LIBPATHS) $(WHOLE_ARCHIVE) $(BUILDDIR)/flisp/libflisp-debug.a $(WHOLE_ARCHIVE) $(BUILDDIR)/support/libsupport-debug.a -ljulia-debug $(RT_LIBS)
CG_DEBUG_LIBS := $(COMMON_LIBPATHS) $(CG_LIBS) -ljulia-debug -ljulia-internal-debug
RT_RELEASE_LIBS := $(COMMON_LIBPATHS) $(WHOLE_ARCHIVE) $(BUILDDIR)/flisp/libflisp.a $(WHOLE_ARCHIVE) $(BUILDDIR)/support/libsupport.a -ljulia $(RT_LIBS)
Expand Down
2 changes: 1 addition & 1 deletion src/datatype.c
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ JL_DLLEXPORT jl_methtable_t *jl_new_method_table(jl_sym_t *name, jl_module_t *mo
jl_atomic_store_relaxed(&mt->cache, jl_nothing);
jl_atomic_store_relaxed(&mt->max_args, 0);
mt->backedges = NULL;
JL_MUTEX_INIT(&mt->writelock);
JL_MUTEX_INIT(&mt->writelock, "methodtable->writelock");
mt->offs = 0;
mt->frozen = 0;
return mt;
Expand Down
4 changes: 2 additions & 2 deletions src/gc.c
Original file line number Diff line number Diff line change
Expand Up @@ -3399,8 +3399,8 @@ void jl_init_thread_heap(jl_ptls_t ptls)
void jl_gc_init(void)
{

JL_MUTEX_INIT(&heapsnapshot_lock);
JL_MUTEX_INIT(&finalizers_lock);
JL_MUTEX_INIT(&heapsnapshot_lock, "heapsnapshot_lock");
JL_MUTEX_INIT(&finalizers_lock, "finalizers_lock");
uv_mutex_init(&gc_cache_lock);
uv_mutex_init(&gc_perm_lock);

Expand Down
14 changes: 13 additions & 1 deletion src/init.c
Original file line number Diff line number Diff line change
Expand Up @@ -705,6 +705,9 @@ static void jl_set_io_wait(int v)
}

extern jl_mutex_t jl_modules_mutex;
extern jl_mutex_t precomp_statement_out_lock;
extern jl_mutex_t newly_inferred_mutex;
extern jl_mutex_t global_roots_lock;

static void restore_fp_env(void)
{
Expand All @@ -717,6 +720,15 @@ static NOINLINE void _finish_julia_init(JL_IMAGE_SEARCH rel, jl_ptls_t ptls, jl_

JL_DLLEXPORT int jl_default_debug_info_kind;

static void init_global_mutexes(void) {
JL_MUTEX_INIT(&jl_modules_mutex, "jl_modules_mutex");
JL_MUTEX_INIT(&precomp_statement_out_lock, "precomp_statement_out_lock");
JL_MUTEX_INIT(&newly_inferred_mutex, "newly_inferred_mutex");
JL_MUTEX_INIT(&global_roots_lock, "global_roots_lock");
JL_MUTEX_INIT(&jl_codegen_lock, "jl_codegen_lock");
JL_MUTEX_INIT(&typecache_lock, "typecache_lock");
}

JL_DLLEXPORT void julia_init(JL_IMAGE_SEARCH rel)
{
// initialize many things, in no particular order
Expand Down Expand Up @@ -746,7 +758,7 @@ JL_DLLEXPORT void julia_init(JL_IMAGE_SEARCH rel)
jl_safepoint_init();
jl_page_size = jl_getpagesize();
htable_new(&jl_current_modules, 0);
JL_MUTEX_INIT(&jl_modules_mutex);
init_global_mutexes();
jl_precompile_toplevel_module = NULL;
ios_set_io_wait_func = jl_set_io_wait;
jl_io_loop = uv_default_loop(); // this loop will internal events (spawning process etc.),
Expand Down
2 changes: 1 addition & 1 deletion src/jl_uv.c
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ void jl_init_uv(void)
{
uv_async_init(jl_io_loop, &signal_async, jl_signal_async_cb);
uv_unref((uv_handle_t*)&signal_async);
JL_MUTEX_INIT(&jl_uv_mutex); // a file-scope initializer can be used instead
JL_MUTEX_INIT(&jl_uv_mutex, "jl_uv_mutex"); // a file-scope initializer can be used instead
}

_Atomic(int) jl_uv_n_waiters = 0;
Expand Down
8 changes: 4 additions & 4 deletions src/julia_locks.h
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ extern "C" {
// The JL_LOCK* and JL_UNLOCK* macros are no-op for non-threading build
// while the jl_mutex_* functions are always locking and unlocking the locks.

JL_DLLEXPORT void _jl_mutex_init(jl_mutex_t *lock, const char *name) JL_NOTSAFEPOINT;
JL_DLLEXPORT void _jl_mutex_wait(jl_task_t *self, jl_mutex_t *lock, int safepoint);
JL_DLLEXPORT void _jl_mutex_lock(jl_task_t *self, jl_mutex_t *lock);
JL_DLLEXPORT int _jl_mutex_trylock_nogc(jl_task_t *self, jl_mutex_t *lock) JL_NOTSAFEPOINT;
Expand Down Expand Up @@ -86,13 +87,12 @@ static inline void jl_mutex_unlock_nogc(jl_mutex_t *lock) JL_NOTSAFEPOINT JL_NOT
_jl_mutex_unlock_nogc(lock);
}

static inline void jl_mutex_init(jl_mutex_t *lock) JL_NOTSAFEPOINT
static inline void jl_mutex_init(jl_mutex_t *lock, const char *name) JL_NOTSAFEPOINT
{
jl_atomic_store_relaxed(&lock->owner, (jl_task_t*)NULL);
lock->count = 0;
_jl_mutex_init(lock, name);
}

#define JL_MUTEX_INIT(m) jl_mutex_init(m)
#define JL_MUTEX_INIT(m, name) jl_mutex_init(m, name)
#define JL_LOCK(m) jl_mutex_lock(m)
#define JL_UNLOCK(m) jl_mutex_unlock(m)
#define JL_LOCK_NOGC(m) jl_mutex_lock_nogc(m)
Expand Down
2 changes: 1 addition & 1 deletion src/method.c
Original file line number Diff line number Diff line change
Expand Up @@ -812,7 +812,7 @@ JL_DLLEXPORT jl_method_t *jl_new_method_uninit(jl_module_t *module)
m->constprop = 0;
m->purity.bits = 0;
m->max_varargs = UINT8_MAX;
JL_MUTEX_INIT(&m->writelock);
JL_MUTEX_INIT(&m->writelock, "method->writelock");
return m;
}

Expand Down
2 changes: 1 addition & 1 deletion src/module.c
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ JL_DLLEXPORT jl_module_t *jl_new_module_(jl_sym_t *name, jl_module_t *parent, ui
m->max_methods = -1;
m->hash = parent == NULL ? bitmix(name->hash, jl_module_type->hash) :
bitmix(name->hash, parent->hash);
JL_MUTEX_INIT(&m->lock);
JL_MUTEX_INIT(&m->lock, "module->lock");
jl_atomic_store_relaxed(&m->bindings, jl_emptysvec);
jl_atomic_store_relaxed(&m->bindingkeyset, (jl_array_t*)jl_an_empty_vec_any);
arraylist_new(&m->usings, 0);
Expand Down
2 changes: 1 addition & 1 deletion src/runtime_ccall.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -362,6 +362,6 @@ JL_GCC_IGNORE_STOP

void jl_init_runtime_ccall(void)
{
JL_MUTEX_INIT(&libmap_lock);
JL_MUTEX_INIT(&libmap_lock, "libmap_lock");
uv_mutex_init(&trampoline_lock);
}
2 changes: 1 addition & 1 deletion src/staticdata.c
Original file line number Diff line number Diff line change
Expand Up @@ -2127,7 +2127,7 @@ static void jl_strip_all_codeinfos(void)
// --- entry points ---

jl_array_t *jl_global_roots_table;
static jl_mutex_t global_roots_lock;
jl_mutex_t global_roots_lock;

JL_DLLEXPORT int jl_is_globally_rooted(jl_value_t *val JL_MAYBE_UNROOTED) JL_NOTSAFEPOINT
{
Expand Down
2 changes: 1 addition & 1 deletion src/staticdata_utils.c
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ static uint64_t jl_worklist_key(jl_array_t *worklist) JL_NOTSAFEPOINT

static jl_array_t *newly_inferred JL_GLOBALLY_ROOTED /*FIXME*/;
// Mutex for newly_inferred
static jl_mutex_t newly_inferred_mutex;
jl_mutex_t newly_inferred_mutex;

// Register array of newly-inferred MethodInstances
// This gets called as the first step of Base.include_package_for_output
Expand Down
47 changes: 47 additions & 0 deletions src/threading.c
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,10 @@
#include "julia_internal.h"
#include "julia_assert.h"

#ifdef USE_ITTAPI
#include "ittapi/ittnotify.h"
#endif

// Ref https://www.uclibc.org/docs/tls.pdf
// For variant 1 JL_ELF_TLS_INIT_SIZE is the size of the thread control block (TCB)
// For variant 2 JL_ELF_TLS_INIT_SIZE is 0
Expand Down Expand Up @@ -724,16 +728,57 @@ JL_DLLEXPORT void jl_exit_threaded_region(void)
}
}

// Profiling stubs

#ifdef USE_ITTAPI

#define profile_lock_init(lock, name) __itt_sync_create(lock, "jl_mutex_t", name, __itt_attr_mutex)
Copy link
Member

Choose a reason for hiding this comment

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

Would it be possible to move these definitions into timing.c/.h?

Ideally this would follow the conventions established there to provide backend-specific implementations (in particular, it should be possible to turn on multiple back-end implementations at once)

Copy link
Member

Choose a reason for hiding this comment

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

I'm also OK if these stay local to threading.c, but I would like to try to support multiple back-ends being enabled at once

Copy link
Member Author

Choose a reason for hiding this comment

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

I believe @vchuravy preferred them as macros here specifically to avoid them showing up in the profile.

Copy link
Member

Choose a reason for hiding this comment

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

Would a static inline function fix that problem?

Copy link
Member

Choose a reason for hiding this comment

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

Yeah... Let's go with the static_inline

#define profile_lock_start_wait(lock) __itt_sync_prepare(lock)
#define profile_lock_acquired(lock) __itt_sync_acquired(lock)
#define profile_lock_release_start(lock) __itt_sync_releasing(lock)
#define profile_lock_release_end(lock)

#endif

#ifndef profile_lock_init
#define profile_lock_init(lock, name)
#endif

#ifndef profile_lock_start_wait
#define profile_lock_start_wait(lock)
#endif

#ifndef profile_lock_acquired
#define profile_lock_acquired(lock)
#endif

#ifndef profile_lock_release_start
#define profile_lock_release_start(lock)
#endif

#ifndef profile_lock_release_end
#define profile_lock_release_end(lock)
#endif

void _jl_mutex_init(jl_mutex_t *lock, const char *name) JL_NOTSAFEPOINT
{
jl_atomic_store_relaxed(&lock->owner, (jl_task_t*)NULL);
lock->count = 0;
profile_lock_init(lock, name);
}

void _jl_mutex_wait(jl_task_t *self, jl_mutex_t *lock, int safepoint)
{
jl_task_t *owner = jl_atomic_load_relaxed(&lock->owner);
if (owner == self) {
lock->count++;
return;
}
profile_lock_start_wait(lock);
while (1) {
if (owner == NULL && jl_atomic_cmpswap(&lock->owner, &owner, self)) {
lock->count = 1;
profile_lock_acquired(lock);
return;
}
if (safepoint) {
Expand Down Expand Up @@ -809,6 +854,7 @@ void _jl_mutex_unlock_nogc(jl_mutex_t *lock)
assert(jl_atomic_load_relaxed(&lock->owner) == jl_current_task &&
"Unlocking a lock in a different thread.");
if (--lock->count == 0) {
profile_lock_release_start(lock);
jl_atomic_store_release(&lock->owner, (jl_task_t*)NULL);
jl_cpu_wake();
if (jl_running_under_rr(0)) {
Expand All @@ -817,6 +863,7 @@ void _jl_mutex_unlock_nogc(jl_mutex_t *lock)
uv_cond_broadcast(&cond);
uv_mutex_unlock(&tls_lock);
}
profile_lock_release_end(lock);
}
#endif
}
Expand Down