Skip to content

Commit cdd599c

Browse files
authored
split a lock currently used for symtab + GC pages + perm alloc (#55257)
Seems fairly odd to have a single lock protecting the symbol table, GC pages and the permanent allocator -- three parts of the codebase which have little interconnection. Let's create a lock for each.
1 parent c49f4aa commit cdd599c

File tree

6 files changed

+20
-13
lines changed

6 files changed

+20
-13
lines changed

src/gc-pages.c

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@
99
extern "C" {
1010
#endif
1111

12+
uv_mutex_t gc_pages_lock;
13+
1214
JL_DLLEXPORT uint64_t jl_get_pg_size(void)
1315
{
1416
return GC_PAGE_SZ;
@@ -67,7 +69,7 @@ char *jl_gc_try_alloc_pages_(int pg_cnt) JL_NOTSAFEPOINT
6769
// more chunks (or other allocations). The final page count is recorded
6870
// and will be used as the starting count next time. If the page count is
6971
// smaller `MIN_BLOCK_PG_ALLOC` a `jl_memory_exception` is thrown.
70-
// Assumes `gc_perm_lock` is acquired, the lock is released before the
72+
// Assumes `gc_pages_lock` is acquired, the lock is released before the
7173
// exception is thrown.
7274
char *jl_gc_try_alloc_pages(void) JL_NOTSAFEPOINT
7375
{
@@ -87,7 +89,7 @@ char *jl_gc_try_alloc_pages(void) JL_NOTSAFEPOINT
8789
block_pg_cnt = pg_cnt = min_block_pg_alloc;
8890
}
8991
else {
90-
uv_mutex_unlock(&gc_perm_lock);
92+
uv_mutex_unlock(&gc_pages_lock);
9193
jl_throw(jl_memory_exception);
9294
}
9395
}
@@ -127,11 +129,11 @@ NOINLINE jl_gc_pagemeta_t *jl_gc_alloc_page(void) JL_NOTSAFEPOINT
127129
goto exit;
128130
}
129131

130-
uv_mutex_lock(&gc_perm_lock);
132+
uv_mutex_lock(&gc_pages_lock);
131133
// another thread may have allocated a large block while we were waiting...
132134
meta = pop_lf_back(&global_page_pool_clean);
133135
if (meta != NULL) {
134-
uv_mutex_unlock(&gc_perm_lock);
136+
uv_mutex_unlock(&gc_pages_lock);
135137
gc_alloc_map_set(meta->data, GC_PAGE_ALLOCATED);
136138
goto exit;
137139
}
@@ -149,7 +151,7 @@ NOINLINE jl_gc_pagemeta_t *jl_gc_alloc_page(void) JL_NOTSAFEPOINT
149151
push_lf_back(&global_page_pool_clean, pg);
150152
}
151153
}
152-
uv_mutex_unlock(&gc_perm_lock);
154+
uv_mutex_unlock(&gc_pages_lock);
153155
exit:
154156
#ifdef _OS_WINDOWS_
155157
VirtualAlloc(meta->data, GC_PAGE_SZ, MEM_COMMIT, PAGE_READWRITE);

src/gc.c

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3979,6 +3979,7 @@ void jl_gc_init(void)
39793979
JL_MUTEX_INIT(&finalizers_lock, "finalizers_lock");
39803980
uv_mutex_init(&page_profile_lock);
39813981
uv_mutex_init(&gc_perm_lock);
3982+
uv_mutex_init(&gc_pages_lock);
39823983
uv_mutex_init(&gc_threads_lock);
39833984
uv_cond_init(&gc_threads_cond);
39843985
uv_sem_init(&gc_sweep_assists_needed, 0);
@@ -4244,7 +4245,7 @@ STATIC_INLINE void *gc_try_perm_alloc_pool(size_t sz, unsigned align, unsigned o
42444245
}
42454246

42464247
// **NOT** a safepoint
4247-
void *jl_gc_perm_alloc_nolock(size_t sz, int zero, unsigned align, unsigned offset)
4248+
void *jl_gc_perm_alloc_nolock(size_t sz, int zero, unsigned align, unsigned offset) JL_NOTSAFEPOINT
42484249
{
42494250
// The caller should have acquired `gc_perm_lock`
42504251
assert(align < GC_PERM_POOL_LIMIT);

src/gc.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -574,6 +574,7 @@ FORCE_INLINE void gc_big_object_link(bigval_t *sentinel_node, bigval_t *node) JL
574574
sentinel_node->next = node;
575575
}
576576

577+
extern uv_mutex_t gc_perm_lock;
577578
extern uv_mutex_t gc_threads_lock;
578579
extern uv_cond_t gc_threads_cond;
579580
extern uv_sem_t gc_sweep_assists_needed;
@@ -593,6 +594,7 @@ void jl_gc_debug_init(void);
593594

594595
// GC pages
595596

597+
extern uv_mutex_t gc_pages_lock;
596598
void jl_gc_init_page(void) JL_NOTSAFEPOINT;
597599
NOINLINE jl_gc_pagemeta_t *jl_gc_alloc_page(void) JL_NOTSAFEPOINT;
598600
void jl_gc_free_page(jl_gc_pagemeta_t *p) JL_NOTSAFEPOINT;

src/init.c

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -746,6 +746,9 @@ JL_DLLEXPORT void julia_init(JL_IMAGE_SEARCH rel)
746746
// Make sure we finalize the tls callback before starting any threads.
747747
(void)jl_get_pgcstack();
748748

749+
// initialize symbol-table lock
750+
uv_mutex_init(&symtab_lock);
751+
749752
// initialize backtraces
750753
jl_init_profile_lock();
751754
#ifdef _OS_WINDOWS_

src/julia_internal.h

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -350,9 +350,6 @@ jl_value_t *jl_gc_small_alloc_noinline(jl_ptls_t ptls, int offset,
350350
int osize);
351351
jl_value_t *jl_gc_big_alloc_noinline(jl_ptls_t ptls, size_t allocsz);
352352
JL_DLLEXPORT int jl_gc_classify_pools(size_t sz, int *osize) JL_NOTSAFEPOINT;
353-
extern uv_mutex_t gc_perm_lock;
354-
void *jl_gc_perm_alloc_nolock(size_t sz, int zero,
355-
unsigned align, unsigned offset) JL_NOTSAFEPOINT;
356353
JL_DLLEXPORT void *jl_gc_perm_alloc(size_t sz, int zero,
357354
unsigned align, unsigned offset) JL_NOTSAFEPOINT;
358355
void gc_sweep_sysimg(void);
@@ -1699,6 +1696,7 @@ void jl_write_malloc_log(void);
16991696
# define jl_unreachable() ((void)jl_assume(0))
17001697
#endif
17011698

1699+
extern uv_mutex_t symtab_lock;
17021700
jl_sym_t *_jl_symbol(const char *str, size_t len) JL_NOTSAFEPOINT;
17031701

17041702
// Tools for locally disabling spurious compiler warnings

src/symbol.c

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
extern "C" {
1616
#endif
1717

18+
uv_mutex_t symtab_lock;
1819
static _Atomic(jl_sym_t*) symtab = NULL;
1920

2021
#define MAX_SYM_LEN ((size_t)INTPTR_MAX - sizeof(jl_taggedvalue_t) - sizeof(jl_sym_t) - 1)
@@ -35,7 +36,7 @@ static jl_sym_t *mk_symbol(const char *str, size_t len) JL_NOTSAFEPOINT
3536
{
3637
jl_sym_t *sym;
3738
size_t nb = symbol_nbytes(len);
38-
jl_taggedvalue_t *tag = (jl_taggedvalue_t*)jl_gc_perm_alloc_nolock(nb, 0, sizeof(void*), 0);
39+
jl_taggedvalue_t *tag = (jl_taggedvalue_t*)jl_gc_perm_alloc(nb, 0, sizeof(void*), 0);
3940
sym = (jl_sym_t*)jl_valueof(tag);
4041
// set to old marked so that we won't look at it in the GC or write barrier.
4142
jl_set_typetagof(sym, jl_symbol_tag, GC_OLD_MARKED);
@@ -86,15 +87,15 @@ jl_sym_t *_jl_symbol(const char *str, size_t len) JL_NOTSAFEPOINT // (or throw)
8687
_Atomic(jl_sym_t*) *slot;
8788
jl_sym_t *node = symtab_lookup(&symtab, str, len, &slot);
8889
if (node == NULL) {
89-
uv_mutex_lock(&gc_perm_lock);
90+
uv_mutex_lock(&symtab_lock);
9091
// Someone might have updated it, check and look up again
9192
if (jl_atomic_load_relaxed(slot) != NULL && (node = symtab_lookup(slot, str, len, &slot))) {
92-
uv_mutex_unlock(&gc_perm_lock);
93+
uv_mutex_unlock(&symtab_lock);
9394
return node;
9495
}
9596
node = mk_symbol(str, len);
9697
jl_atomic_store_release(slot, node);
97-
uv_mutex_unlock(&gc_perm_lock);
98+
uv_mutex_unlock(&symtab_lock);
9899
}
99100
return node;
100101
}

0 commit comments

Comments
 (0)