Skip to content

Commit 68d62ab

Browse files
authored
Revert "gc: avoid cpu stalls when starting" (JuliaLang#45779)
Reverts 6e8c78a from JuliaLang#45561 because it seems to be causing deadlock on CI. I suspect this logic is missing a uv_cond_broadcast for safe-region transitions of ptls->gc_state.
1 parent a2cc0f4 commit 68d62ab

File tree

3 files changed

+16
-28
lines changed

3 files changed

+16
-28
lines changed

src/gc.c

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -190,7 +190,22 @@ NOINLINE uintptr_t gc_get_stack_ptr(void)
190190

191191
#define should_timeout() 0
192192

193-
void jl_gc_wait_for_the_world(void);
193+
static void jl_gc_wait_for_the_world(void)
194+
{
195+
if (jl_n_threads > 1)
196+
jl_wake_libuv();
197+
for (int i = 0; i < jl_n_threads; i++) {
198+
jl_ptls_t ptls2 = jl_all_tls_states[i];
199+
// This acquire load pairs with the release stores
200+
// in the signal handler of safepoint so we are sure that
201+
// all the stores on those threads are visible.
202+
// We're currently also using atomic store release in mutator threads
203+
// (in jl_gc_state_set), but we may want to use signals to flush the
204+
// memory operations on those threads lazily instead.
205+
while (!jl_atomic_load_relaxed(&ptls2->gc_state) || !jl_atomic_load_acquire(&ptls2->gc_state))
206+
jl_cpu_pause(); // yield?
207+
}
208+
}
194209

195210
// malloc wrappers, aligned allocation
196211

src/safepoint.c

Lines changed: 0 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -109,31 +109,6 @@ void jl_safepoint_init(void)
109109
jl_safepoint_pages = addr;
110110
}
111111

112-
void jl_gc_wait_for_the_world(void)
113-
{
114-
assert(jl_n_threads);
115-
if (jl_n_threads > 1)
116-
jl_wake_libuv();
117-
for (int i = 0; i < jl_n_threads; i++) {
118-
jl_ptls_t ptls2 = jl_all_tls_states[i];
119-
// This acquire load pairs with the release stores
120-
// in the signal handler of safepoint so we are sure that
121-
// all the stores on those threads are visible.
122-
// We're currently also using atomic store release in mutator threads
123-
// (in jl_gc_state_set), but we may want to use signals to flush the
124-
// memory operations on those threads lazily instead.
125-
while (!jl_atomic_load_relaxed(&ptls2->gc_state) || !jl_atomic_load_acquire(&ptls2->gc_state)) {
126-
// Use system mutexes rather than spin locking to minimize wasted CPU time
127-
// while we wait for other threads reach a safepoint.
128-
// This is particularly important when run under rr.
129-
uv_mutex_lock(&safepoint_lock);
130-
if (!jl_atomic_load_relaxed(&ptls2->gc_state))
131-
uv_cond_wait(&safepoint_cond, &safepoint_lock);
132-
uv_mutex_unlock(&safepoint_lock);
133-
}
134-
}
135-
}
136-
137112
int jl_safepoint_start_gc(void)
138113
{
139114
if (jl_n_threads == 1) {
@@ -185,7 +160,6 @@ void jl_safepoint_wait_gc(void)
185160
{
186161
// The thread should have set this is already
187162
assert(jl_atomic_load_relaxed(&jl_current_task->ptls->gc_state) != 0);
188-
uv_cond_broadcast(&safepoint_cond);
189163
// Use normal volatile load in the loop for speed until GC finishes.
190164
// Then use an acquire load to make sure the GC result is visible on this thread.
191165
while (jl_atomic_load_relaxed(&jl_gc_running) || jl_atomic_load_acquire(&jl_gc_running)) {

src/signals-mach.c

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,6 @@ static int jl_mach_gc_wait(jl_ptls_t ptls2,
8585
arraylist_push(&suspended_threads, (void*)item);
8686
thread_suspend(thread);
8787
uv_mutex_unlock(&safepoint_lock);
88-
uv_cond_broadcast(&safepoint_cond);
8988
return 1;
9089
}
9190

0 commit comments

Comments
 (0)