Per-thread cpu time by d-netto · Pull Request #181 · RelationalAI/julia

d-netto · 2024-09-20T15:37:29Z

PR Description

Expands on #175 by:

Adding per-thread start-time.
Using a proper macro/API to increment metrics.
Splitting GC time into safe-point time and proper GC time.
Introducing internal SpinLock time.
Introducing feature flag to report metrics in terms of CPU time or wall time.

Checklist

Requirements for merging:

I have opened an issue or PR upstream on JuliaLang/julia: <link to JuliaLang/julia>
I have removed the port-to-* labels that don't apply.
I have opened a PR on raicode to test these changes:

nickrobinson251

nice work. Have you been able to test this out on some simple examples to sense-check the numbers?

base/timing.jl

src/julia_locks.h

src/threading.c

base/timing.jl

src/threading.c

NHDaly · 2024-09-25T19:07:52Z

src/partr.c

+JL_DLLEXPORT jl_task_t *jl_task_get_next(jl_value_t *trypoptask, jl_value_t *q, jl_value_t *checkempty)
+{
+    uint64_t t0 = jl_record_time_for_tls_metric();
+    jl_task_t *task = _jl_task_get_next(trypoptask, q, checkempty);
+    jl_increment_timing_tls_metric(jl_current_task->ptls, scheduler_time, jl_record_time_for_tls_metric() - t0);
+    return task;
+}


I think this got lost in the handoff, but the original plan was to not record the current time twice for the "fast-path" of task switching.

The concern is that adding two syscalls can meaningfully slow down task switching, and the idea was that we probably don't need to do this anyway since the time we'd be measuring in the fast-path case is trivial (we believe).

So ideally I think we'd want to only do this in the slow paths inside jl_task_get_next?

BUT to be honest, I find your approach much cleaner and clearer - and it's nice that we would get an accurate measure of that switch time which is supposed to be cheap......... AND we are going to need something like this anyway for the per-task CPU time in https://relationalai.atlassian.net/browse/RAI-30482...

So maybe if we could solve the vDSO / fast instruction problem, then we could do it this way after all?
Something to resolve CC @kpamnany.

github-actions · 2024-11-01T00:11:50Z

This PR is stale because it has been open 30 days with no activity. Comment or remove stale label, or this PR will be closed in 5 days.

@nalimilan

…ang#57266) Stdlib: Statistics URL: https://github.com/JuliaStats/Statistics.jl.git Stdlib branch: master Julia branch: master Old commit: d49c2bf New commit: 77bd570 Julia version: 1.13.0-DEV Statistics version: 1.11.2(Does not match) Bump invoked by: @nalimilan Powered by: [BumpStdlibs.jl](https://github.com/JuliaLang/BumpStdlibs.jl) Diff: JuliaStats/Statistics.jl@d49c2bf...77bd570 ``` $ git log --oneline d49c2bf..77bd570 77bd570 Fix `quantile` doctest (#188) bfa5c6b Merge pull request #184 from JuliaStats/an/quantilemuladd 6bd1531 Update src/Statistics.jl 44d51c7 Use muladd in aleph calculation in _quantile to avoid some rounding errors. 793733e Bump codecov/codecov-action from 4 to 5 (#181) ``` Co-authored-by: nalimilan <1120448+nalimilan@users.noreply.github.com>

@nalimilan

…ang#57266) Stdlib: Statistics URL: https://github.com/JuliaStats/Statistics.jl.git Stdlib branch: master Julia branch: master Old commit: d49c2bf New commit: 77bd570 Julia version: 1.13.0-DEV Statistics version: 1.11.2(Does not match) Bump invoked by: @nalimilan Powered by: [BumpStdlibs.jl](https://github.com/JuliaLang/BumpStdlibs.jl) Diff: JuliaStats/Statistics.jl@d49c2bf...77bd570 ``` $ git log --oneline d49c2bf..77bd570 77bd570 Fix `quantile` doctest (#188) bfa5c6b Merge pull request #184 from JuliaStats/an/quantilemuladd 6bd1531 Update src/Statistics.jl 44d51c7 Use muladd in aleph calculation in _quantile to avoid some rounding errors. 793733e Bump codecov/codecov-action from 4 to 5 (#181) ``` Co-authored-by: nalimilan <1120448+nalimilan@users.noreply.github.com> (cherry picked from commit 504cbc3)

@DilumAluthge

Stdlib: Distributed URL: https://github.com/JuliaLang/Distributed.jl Stdlib branch: master Julia branch: master Old commit: cd92195 New commit: d439c24 Julia version: 1.14.0-DEV Distributed version: 1.11.0 (Does not match) Bump invoked by: @DilumAluthge Powered by: [BumpStdlibs.jl](https://github.com/JuliaLang/BumpStdlibs.jl) Diff: JuliaLang/Distributed.jl@cd92195...d439c24 ``` $ git log --oneline cd92195..d439c24 d439c24 CI: Add a CI job that runs JET tests (#164) d396a8c Fix some JET errors around matching methods for `send_connection_hdr(...)` and `send_msg_now(...)` (#180) 231da28 Fix some JET errors around matching methods for `kill(...)` and `process_exited(...)` (#172) 2adcd26 Rename one method of `run_work_thunk()` to `run_work_thunk_remotevalue()`; this fixes a JET error around matching methods for `run_work_thunk(...)` (#181) 1bc91f9 Fix a JET error regarding the existence of the local variable `reducer` at a certain point (#169) b7c43b2 Fix a JET error around matching methods for `push!(...)` (#173) d06aa73 Fix a JET error around matching methods for `getindex(...)` (#170) 0cf9910 Fix a JET error around matching methods for `read_worker_host_port(...)` (#171) 9f6459f Fix JET errors around matching methods for `lock(...)` and `unlock(...)` (#167) 9724553 Fix a JET error around matching methods for `shell_escape_wincmd(...)` (#174) d65a996 Fix a JET error by narrowing the type signature of the `create_worker(x, y)` function (#175) 3ebddd3 avoid `Core.Box` in the package (#161) 2fe1aa4 Change `Distributed.cluster_manager` from a global non-constant `ClusterManager` to a global constant `Ref{ClusterManager}` (#177) 6649a94 Ensure correct type parameter of serialized RemoteChannel (#179) 871e3d7 Fix JET errors around matching methods for `send_msg_now(...)` (#165) 56fa9f2 Fix JET errors around matching methods for `send_msg(...)` (#166) a6195c0 Bump actions/checkout from 5 to 6 (#155) ``` Co-authored-by: DilumAluthge <5619885+DilumAluthge@users.noreply.github.com>

@DilumAluthge

Stdlib: Tar URL: https://github.com/JuliaIO/Tar.jl.git Stdlib branch: master Julia branch: master Old commit: e15495e New commit: 6a61f5c Julia version: 1.14.0-DEV Tar version: 1.10.0 (Does not match) Bump invoked by: @DilumAluthge Powered by: [BumpStdlibs.jl](https://github.com/JuliaLang/BumpStdlibs.jl) Diff: JuliaIO/Tar.jl@e15495e...6a61f5c ``` $ git log --oneline e15495e..6a61f5c 6a61f5c Bump codecov/codecov-action from 4 to 6 (#181) 1a1b323 Bump julia-actions/cache from 2 to 3 (#193) ``` Co-authored-by: DilumAluthge <5619885+DilumAluthge@users.noreply.github.com>

github-actions bot added port-to-v1.10 port-to-v1.12 This change should apply to Julia v1.12 builds labels Sep 20, 2024

d-netto requested review from kpamnany and nickrobinson251 September 20, 2024 15:49

d-netto force-pushed the dcn-user-cpu-time branch 3 times, most recently from 4e1bf4c to 296eeb4 Compare September 23, 2024 16:40

nickrobinson251 reviewed Sep 23, 2024

View reviewed changes

base/timing.jl Outdated Show resolved Hide resolved

src/julia_locks.h Show resolved Hide resolved

nickrobinson251 reviewed Sep 23, 2024

View reviewed changes

src/threading.c Outdated Show resolved Hide resolved

NHDaly reviewed Sep 23, 2024

View reviewed changes

base/timing.jl Outdated Show resolved Hide resolved

NHDaly reviewed Sep 23, 2024

View reviewed changes

base/timing.jl Outdated Show resolved Hide resolved

NHDaly reviewed Sep 23, 2024

View reviewed changes

src/threading.c Outdated Show resolved Hide resolved

d-netto force-pushed the dcn-user-cpu-time branch from 296eeb4 to 898fffb Compare September 24, 2024 20:08

NHDaly reviewed Sep 25, 2024

View reviewed changes

d-netto force-pushed the dcn-user-cpu-time branch 5 times, most recently from abe0d2e to d7c0b4c Compare September 30, 2024 18:38

per-thread user time

1cbe632

d-netto force-pushed the dcn-user-cpu-time branch from d7c0b4c to 1cbe632 Compare October 1, 2024 13:28

github-actions bot added the stale This pull request is inactive label Nov 1, 2024

github-actions bot closed this Nov 7, 2024

d-netto removed the port-to-v1.12 This change should apply to Julia v1.12 builds label Jan 30, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Per-thread cpu time#181

Per-thread cpu time#181
d-netto wants to merge 1 commit intov1.10.2+RAIfrom
dcn-user-cpu-time

d-netto commented Sep 20, 2024

Uh oh!

nickrobinson251 left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

NHDaly Sep 25, 2024

Uh oh!

github-actions bot commented Nov 1, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

d-netto commented Sep 20, 2024

PR Description

Checklist

Uh oh!

nickrobinson251 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

NHDaly Sep 25, 2024

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Nov 1, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants