Skip to content

Commit 4fbd122

Browse files
gadmmgadmm
committed
Do not wait that other threads have processed signals not for us in [caml_enter_blocking_section]
1 parent 2ead128 commit 4fbd122

File tree

3 files changed

+32
-18
lines changed

3 files changed

+32
-18
lines changed

‎runtime/caml/domain.h‎

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -37,12 +37,13 @@ extern "C" {
3737
#define Max_domains 16
3838
#endif
3939

40-
/* is the minor heap full or an external interrupt has been triggered */
41-
Caml_inline int caml_check_gc_interrupt(caml_domain_state * dom_st)
40+
/* is the minor heap full or has an external interrupt been triggered? */
41+
Caml_inline int caml_check_gc_interrupt(uintnat young_ptr,
42+
caml_domain_state * dom_st)
4243
{
4344
CAMLalloc_point_here;
4445
uintnat young_limit = atomic_load_relaxed(&dom_st->young_limit);
45-
if ((uintnat)dom_st->young_ptr < young_limit) {
46+
if (young_ptr < young_limit) {
4647
/* Synchronise for the case when [young_limit] was used to interrupt
4748
us. */
4849
atomic_thread_fence(memory_order_acquire);
@@ -51,8 +52,13 @@ Caml_inline int caml_check_gc_interrupt(caml_domain_state * dom_st)
5152
return 0;
5253
}
5354

54-
#define Caml_check_gc_interrupt(dom_st) \
55-
(CAMLunlikely(caml_check_gc_interrupt(dom_st)))
55+
/* Interrupt functions */
56+
#define INTERRUPT_EXTERNAL ((uintnat)-1)
57+
58+
#define Caml_check_gc_interrupt(dom_st) \
59+
(CAMLunlikely(caml_check_gc_interrupt((uintnat)dom_st->young_ptr, dom_st)))
60+
#define Caml_check_gc_external_interrupt(dom_st) \
61+
(CAMLunlikely(caml_check_gc_interrupt(INTERRUPT_EXTERNAL - 1, dom_st)))
5662

5763
asize_t caml_norm_minor_heap_size (intnat);
5864
int caml_reallocate_minor_heap(asize_t);

‎runtime/domain.c‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -291,7 +291,7 @@ Caml_inline void interrupt_domain(struct interruptor* s)
291291
{
292292
atomic_uintnat * interrupt_word =
293293
atomic_load_explicit(&s->interrupt_word, memory_order_relaxed);
294-
atomic_store_rel(interrupt_word, (uintnat)(-1));
294+
atomic_store_rel(interrupt_word, INTERRUPT_EXTERNAL);
295295
}
296296

297297
int caml_incoming_interrupts_queued(void)
@@ -1464,16 +1464,18 @@ void caml_interrupt_all_for_signal(void)
14641464

14651465
void caml_reset_young_limit(caml_domain_state * dom_st)
14661466
{
1467-
/* An interrupt might have been queued in the meanwhile; this
1468-
achieves the proper synchronisation. */
1467+
/* An interrupt might have been queued in the meanwhile; the
1468+
atomic_exchange achieves the proper synchronisation by reading
1469+
the previous value. */
14691470
atomic_exchange(&dom_st->young_limit, (uintnat)dom_st->young_start);
14701471
dom_internal * d = &all_domains[dom_st->id];
14711472
if (atomic_load_relaxed(&d->interruptor.interrupt_pending)
14721473
|| dom_st->requested_minor_gc
14731474
|| dom_st->requested_major_slice
14741475
|| atomic_load_relaxed(&dom_st->requested_external_interrupt)
14751476
|| dom_st->action_pending) {
1476-
atomic_store_rel(&dom_st->young_limit, (uintnat)-1);
1477+
/* We distinguish fresh interrupts from ones we are delaying. */
1478+
atomic_store_relaxed(&dom_st->young_limit, (uintnat)dom_st->young_end + 1);
14771479
CAMLassert(caml_check_gc_interrupt(dom_st));
14781480
}
14791481
}

‎runtime/signals.c‎

Lines changed: 15 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -163,16 +163,20 @@ CAMLexport void (*caml_leave_blocking_section_hook)(void) =
163163

164164
CAMLexport void caml_enter_blocking_section(void)
165165
{
166-
while (1){
166+
caml_domain_state * domain = Caml_state;
167+
while (1) {
167168
/* Process all pending signals now */
168-
caml_raise_if_exception(caml_process_pending_signals_exn());
169+
if (Caml_check_gc_interrupt(domain)) {
170+
/* Some actions might remain after this, but it is no longer an
171+
external interrupt. */
172+
caml_set_action_pending(domain);
173+
caml_handle_gc_interrupt();
174+
caml_raise_if_exception(caml_process_pending_signals_exn());
175+
}
169176
caml_enter_blocking_section_hook ();
170-
/* Check again for pending signals.
171-
If none, done; otherwise, try again */
172-
// FIXME: does this become very slow if a signal is recorded but
173-
// is masked for everybody in capacity of running signals at this
174-
// point?
175-
if (!caml_check_pending_signals()) break;
177+
/* Check again if a signal arrived in the meanwhile. If none, done;
178+
otherwise, try again */
179+
if (!Caml_check_gc_external_interrupt(domain)) break;
176180
caml_leave_blocking_section_hook ();
177181
}
178182
}
@@ -301,7 +305,9 @@ void caml_request_minor_gc (void)
301305
void caml_set_action_pending(caml_domain_state * dom_st)
302306
{
303307
dom_st->action_pending = 1;
304-
atomic_store_rel(&dom_st->young_limit, (uintnat)-1);
308+
/* Non-external interrupt */
309+
atomic_store_rel(&dom_st->young_limit, (uintnat)dom_st->young_end + 1);
310+
CAMLassert(caml_check_gc_interrupt(dom_st));
305311
}
306312

307313
CAMLexport int caml_check_pending_actions(void)

0 commit comments

Comments
 (0)