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

profiling: add Task/GC/Inference/Codegen signposts for Tracy #49140

Merged
merged 12 commits into from
Apr 17, 2023

Conversation

topolarity
Copy link
Member

@topolarity topolarity commented Mar 24, 2023

This adds a variety of metadata to send to Tracy during profiling, in particular method instance signatures, the Heap size, GC Mark/Sweep/Stop-the-World, and a couple of other small pieces of relevant metadata.

It also adds full support for Tasks, tracking these as "Fibers" on the Tracy side:

Check out an example trace here: https://topolarity.github.io/tracy-traces/tracy-experimental/

image

Loose ends

Before this is ready for final review, I still need to:

  • Finalize the JL_TIMING API changes
  • Resolve incompatibility between TRACY_ON_DEMAND and Tracy's Fiber support

This includes a few separate enhancements:
  1. `growable` is added to allow for fixed-size buffers
  2. `ios_t` struct layout is optimized for better packing
  3. inline buffer is increased to 83 bytes from 54

This change grows the size of ios_t by 16 bytes (160 -> 172 bytes), but
the internal buffer was increased by 29 bytes thanks to re-arranging the
fields in the struct to be packed more efficiently.
@topolarity topolarity force-pushed the tracy-experimental branch 4 times, most recently from 8d97522 to 5fd96e1 Compare April 5, 2023 19:39
@topolarity topolarity requested a review from staticfloat April 5, 2023 19:39
@topolarity topolarity marked this pull request as ready for review April 5, 2023 19:39
@topolarity topolarity force-pushed the tracy-experimental branch from 8b7adce to 2cc62d0 Compare April 5, 2023 21:45
@topolarity topolarity requested a review from gbaraldi April 5, 2023 21:46
This adds a variety of metadata to send to Tracy during profiling, in
particular method instance signatures, the Heap size, and a couple of
other small pieces of relevant metadata.

It also adds full support for Task execution, tracking these as "Fibers"
on the Tracy side. This does incur some overhead until an enhancement
is implemented on the Tracy-side to avoid a globally synchronous Fiber
event queue, but most of the events that we're tracking now are
relatively coarse so it should not be a major issue.
This environmental variable instructs the Julia runtime to wait until
it receives a connection from the Tracy profiler before starting.

This allows us to get a complete trace, without having to accumulate a
large queue of events to send over the wire later.
This functionality is currently only exposed on the master branch of
Tracy, but it's quite useful so let's carry the patch to expose it until
the next Tracy release.
This is useful for Tracy, which can receive method signature strings
over the write. For performance reasons, we want to keep those strings
as small as possible, typically less than 80 characters long.

This change adds basic "configuration" support for `jl_static_show`,
including only a "quiet" parameter for now which can be used to avoid
printing DataType parameters.
This replaces the `ENABLE_TIMINGS` #define in `options.h`

Since we now support multiple back-ends for timing events, each back-end
is expected to be enabled with a `WITH_*` Makefile flag, and then the
JL_TIMING API is automatically enabled if any back-end is available.

Turning on multiple back-ends at the same time is also supported.
Cap the number of method instances we report to the profiler for a
single LLVM_MODULE_FINISH to 10.
@topolarity topolarity force-pushed the tracy-experimental branch from 2cc62d0 to 1526534 Compare April 6, 2023 14:33
@topolarity topolarity requested a review from kpamnany April 7, 2023 13:43
src/jitlayers.cpp Outdated Show resolved Hide resolved
src/jlapi.c Outdated Show resolved Hide resolved
src/julia.h Show resolved Hide resolved
Copy link
Contributor

@kpamnany kpamnany left a comment

Choose a reason for hiding this comment

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

Looks fantastic!

Another useful point to instrument might be thread sleep/wakeup; search for PROBE in partr.c.

src/timing.c Outdated Show resolved Hide resolved
src/julia.h Show resolved Hide resolved
src/task.c Outdated Show resolved Hide resolved
src/jlapi.c Outdated Show resolved Hide resolved
src/jlapi.c Show resolved Hide resolved
This enables broadcast messages so that Tracy processes can be listed
by the profiler, which is a nice quality-of-life feature.
This commit adds the following environment variables:
  - `JULIA_TIMING_SUBSYSTEMS` which can be set to, e.g.,
    "+INFERENCE,-GC,METHOD_MATCH" to enable INFERENCE
    and METHOD_MATCH and disable GC.
  - `JULIA_TIMING_METADATA_PRINT_LIMIT` which defaults
    to 10 and determines how many metadata items to add to
    a single timing zone before truncating

This commit also includes other miscellaneous changes to incorporate
review feedback.
@topolarity
Copy link
Member Author

Another useful point to instrument might be thread sleep/wakeup; search for PROBE in partr.c.

Great idea - I've added that to my list for the follow-up PR (which is targeting locks, etc.)

One other detail that I noticed today is that JULIA_WAIT_FOR_TRACY interacts poorly with precompilation. The spawned Julia processes will see the environment variable, also wait for a Tracy connection on start-up, and will make no progress.

Any ideas about how we should handle that?

@staticfloat
Copy link
Member

I would just delete that environment variable by adding a mapping to nothing here.

We'll eventually want to provide some kind of useful interface to turn
this on for a particular pre-compilation, but for now we should just
prevent causing pre-compilation to hang forever.
@topolarity topolarity requested a review from staticfloat April 17, 2023 17:50
@staticfloat staticfloat added the merge me PR is reviewed. Merge when all tests are passing label Apr 17, 2023
@staticfloat staticfloat merged commit 386b09f into JuliaLang:master Apr 17, 2023
@carstenbauer
Copy link
Member

Great stuff! However, I noticed that this PR doesn't add any documentation for the new features. I think that would be a very valuable addition.

@oscardssmith oscardssmith added profiler and removed merge me PR is reviewed. Merge when all tests are passing labels Apr 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants