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

WIP thread cpu time #175

Closed
wants to merge 10 commits into from
Prev Previous commit
Next Next commit
WIP time task-switch and sleep
  • Loading branch information
nickrobinson251 committed Aug 28, 2024
commit 14099bbdf32f95859726b97b1a41823eb704b383
1 change: 1 addition & 0 deletions src/julia_threads.h
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,7 @@ struct _jl_bt_element_t;
typedef struct {
uint64_t start_time;
uint64_t sleep_time;
uint64_t scheduler_time;
uint64_t lock_spin_time;
Copy link
Member Author

Choose a reason for hiding this comment

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

to do: add compile_time (which could be a subset of lock_spin_time i guess or we could stop accumulating lock_spin_time when compilation_time starts?)

in future maybe we could split lock_spin_time to have timing for a few important internal locks (like the codegen_lock) but i think that can be follow-up work?

/* uint64_t lock_spin_entry; // needed to time Threads.SpinLock */
uint64_t gc_time;
Expand Down
13 changes: 9 additions & 4 deletions src/partr.c
Original file line number Diff line number Diff line change
Expand Up @@ -389,12 +389,14 @@ JL_DLLEXPORT jl_task_t *jl_task_get_next(jl_value_t *trypoptask, jl_value_t *q,
{
jl_task_t *ct = jl_current_task;
uint64_t start_cycles = 0;

uint64_t t0 = jl_hrtime();
while (1) {
jl_ptls_t ptls = ct->ptls;
jl_task_t *task = get_next_task(trypoptask, q);
if (task)
if (task) {
ptls->timing_tls.scheduler_time += jl_hrtime() - t0;
return task;

}
Copy link
Member Author

Choose a reason for hiding this comment

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

this is the fast-path for task-switches (i think?), but i think we've concluded this shouldn't add too much overhead (given jl_hrtime is a vdso call)... still need to verify that experimentally though

// quick, race-y check to see if there seems to be any stuff in there
jl_cpu_pause();
if (!check_empty(checkempty)) {
Expand All @@ -403,7 +405,6 @@ JL_DLLEXPORT jl_task_t *jl_task_get_next(jl_value_t *trypoptask, jl_value_t *q,
}

jl_cpu_pause();
jl_ptls_t ptls = ct->ptls;
if (sleep_check_after_threshold(&start_cycles) || (ptls->tid == 0 && (!jl_atomic_load_relaxed(&_threadedregion) || wait_empty))) {
// acquire sleep-check lock
jl_atomic_store_relaxed(&ptls->sleep_check_state, sleeping);
Expand All @@ -425,6 +426,7 @@ JL_DLLEXPORT jl_task_t *jl_task_get_next(jl_value_t *trypoptask, jl_value_t *q,
JL_PROBE_RT_SLEEP_CHECK_TASK_WAKE(ptls);
}
if (task)
ptls->timing_tls.scheduler_time += jl_hrtime() - t0;
return task;
continue;
}
Expand All @@ -433,6 +435,7 @@ JL_DLLEXPORT jl_task_t *jl_task_get_next(jl_value_t *trypoptask, jl_value_t *q,
jl_atomic_store_relaxed(&ptls->sleep_check_state, not_sleeping); // let other threads know they don't need to wake us
JL_PROBE_RT_SLEEP_CHECK_TASK_WAKE(ptls);
}
ptls->timing_tls.scheduler_time += jl_hrtime() - t0;
return task;
}

Expand Down Expand Up @@ -507,6 +510,7 @@ JL_DLLEXPORT jl_task_t *jl_task_get_next(jl_value_t *trypoptask, jl_value_t *q,

// the other threads will just wait for an individual wake signal to resume
JULIA_DEBUG_SLEEPWAKE( ptls->sleep_enter = cycleclock() );
uint64_t tsleep0 = jl_hrtime();
int8_t gc_state = jl_gc_safe_enter(ptls);
uv_mutex_lock(&ptls->sleep_lock);
while (may_sleep(ptls)) {
Expand All @@ -523,6 +527,7 @@ JL_DLLEXPORT jl_task_t *jl_task_get_next(jl_value_t *trypoptask, jl_value_t *q,
assert(jl_atomic_load_relaxed(&ptls->sleep_check_state) == not_sleeping);
uv_mutex_unlock(&ptls->sleep_lock);
JULIA_DEBUG_SLEEPWAKE( ptls->sleep_leave = cycleclock() );
ptls->timing_tls.sleep_time += jl_hrtime() - tsleep0;
Copy link
Member Author

Choose a reason for hiding this comment

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

currently sleep_time is a subset of scheduler_time (so we might want to rename scheduler_time to make that clear, or do the extra accounting so that we stop accumulating scheduler_time when we start accumulating sleep_time?)

jl_gc_safe_leave(ptls, gc_state); // contains jl_gc_safepoint
start_cycles = 0;
if (task) {
Expand Down