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.

Sign up for GitHub

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

Jump to bottom

[BUG] conda package missing legion_prof #950

Closed
syamajala opened this issue Jul 15, 2024 · 21 comments
Closed

[BUG] conda package missing legion_prof #950

syamajala opened this issue Jul 15, 2024 · 21 comments
Assignees
@m3vaz @mag1cp1n
Labels
category:task PR is a simple task and will not be included in release notes

Comments

@syamajala
Copy link

Software versions

The legate 24.06.00 conda package is missing legion_prof

Jupyter notebook / Jupyter Lab version

No response

Expected behavior

The conda packages should include legion_prof

Observed behavior

legion_prof is missing

Example code or instructions

Install legate 24.06.00 from conda.

Stack traceback or browser console output

No response
@marcinz
Copy link
Collaborator

marcinz commented Jul 17, 2024

To get the profiler, we would need to configure with --legion-rust-profiler. This would increase the footprint of the package by requiring rust. @m3vaz, I am thinking that we should add this as a separate package output to our recipe.

@syamajala
Copy link
Author

Do you know what legion commit was used to build legate 24.06.00?

I can just install the profiler myself for now, but when I tried the latest legion master the profiler was crashing when I tried to generate a profile from a log generated with legate 24.06.00.

@marcinz
Copy link
Collaborator

marcinz commented Jul 17, 2024

@syamajala v24.06.00 was built of 75074815f2cc063bd38f78901a7538a06012fe43.

@m3vaz, I guess for the time being we could add the profiler to the package and worry about splitting later. We would need to add a rust dependency, but that should probably be enough.

@syamajala
Copy link
Author

Thanks! Im able to view the profiles after checking out that commit of legion.

@manopapad
Copy link
Contributor

manopapad commented Jul 18, 2024 •
edited
Loading

Note that adding the rust profiler unconditionally to all CI builds will result in an uncachable rust build (at least when using ccache, which doesn't support rust) (sccache appears to support rust).

Also I have suggested that the build of the rust profiler happen in a separate step, rather than as part of the main build, otherwise we hit #860. See internal work item LLRDO-176.

@syamajala
Copy link
Author

I am only able to use legion_prof --view trying to generate an archive crashes.

@syamajala
Copy link
Author

stack trace for legion_prof --archive crash:

Reading log file "legate_0.prof"...
Matched 98916 objects
No Legion Spy data, skipping postprocess step
Sorting time ranges
Created output directory "legion_prof.2"
Writing level 0 with 1 tiles
thread '<unnamed>' panicked at src/backend/data_source.rs:1137:67:
called `Option::unwrap()` on a `None` value
stack backtrace:
thread '<unnamed>' panicked at src/backend/data_source.rs:1137:67:
called `Option::unwrap()` on a `None` value
   0: rust_begin_unwind
             at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/std/src/panicking.rs:645:5
   1: core::panicking::panic_fmt
             at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/core/src/panicking.rs:72:14
   2: core::panicking::panic
             at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/core/src/panicking.rs:144:5
   3: <legion_prof::backend::data_source::StateDataSource as legion_prof_viewer::data::DataSource>::fetch_slot_meta_tile
   4: std::panicking::try
   5: rayon_core::registry::Registry::catch_unwind
   6: <rayon_core::job::HeapJob<BODY> as rayon_core::job::Job>::execute
   7: rayon_core::registry::WorkerThread::wait_until_cold
   8: rayon_core::registry::ThreadBuilder::run
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.
stack backtrace:
Rayon: detected unexpected panic; aborting
[1]    607787 IOT instruction (core dumped)  RUST_BACKTRACE=1 legion_prof --archive legate_0.prof

@syamajala
Copy link
Author

24.06.01 does not seem to have the profiler either?

@syamajala
Copy link
Author

what commit of legion was 24.06.01 built with? i tried the latest legion master but the profiler is crashing when i try to view cunumeric profile logs.

@manopapad
Copy link
Contributor

It used Legion commit a66da82b8fb1fb45d3605963cb33bced51da2f6e

@syamajala
Copy link
Author

what commit of legion was legate 24.09.00.dev236 built with? i tried the profiler from master and it opens the profile but then it crashes when i try to look at anything.

@manopapad
Copy link
Contributor

Looks like c032dab254f423ccab36d05c47fed42b94f0b3f5, @marcinz @sandeepd-nv @m3vaz is that right?

@manopapad
Copy link
Contributor

Profiler packages are now being created, still need to make sure they get uploaded. Targeting by code freeze.

@syamajala
Copy link
Author

Theres still issues with profiling. Hitting this: StanfordLegion/legion#1773

@m3vaz
Copy link

m3vaz commented Oct 15, 2024

@manopapad is there any AI on us here?

@syamajala
Copy link
Author

Profiling is working again so this can proceed now I think.

@syamajala
Copy link
Author

We need at least this legion commit: https://gitlab.com/StanfordLegion/legion/-/commit/e1e018f71e0127405d44b42dae6ea5be8f48fe54

@m3vaz m3vaz added the category:task PR is a simple task and will not be included in release notes label Oct 15, 2024
@manopapad
Copy link
Contributor

@manopapad is there any AI on us here?

We should start uploading the profiler builds whenever we push a nightly build to the public label. I don't see one from yesterday's public nightly push.

@m3vaz
Copy link

m3vaz commented Oct 15, 2024

Ack, it sounds like we'll need to push another upload before the end of the week. I can include the profiler packages in the next push.

@manopapad
Copy link
Contributor

Working through build issues, planning to have a nightly push by EOW, that should include the profiler build, once @syamajala confirms we can close this.

@manopapad
Copy link
Contributor

Latest nightly drop contains the legate profiler package, in package legate-profiler.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@m3vaz m3vaz

@mag1cp1n mag1cp1n

Labels
category:task PR is a simple task and will not be included in release notes
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
6 participants
@syamajala @manopapad @marcinz @m3vaz @sandeepd-nv @mag1cp1n