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

Add paging by thread (and task, optionally) #176

Merged
merged 16 commits into from
Jan 14, 2022

Conversation

IanButterworth
Copy link
Collaborator

@IanButterworth IanButterworth commented Oct 30, 2021

Depends on

julia> using ProfileView

julia> function myfunc()
                  A = rand(400, 400, 400)
                  maximum(A)
              end
myfunc (generic function with 1 method)

julia> myfunc();

julia> @profview myfunc()

ezgif com-gif-maker (1)

@timholy
Copy link
Owner

timholy commented Oct 30, 2021

5sdak4

(Not real ones, anyway. I should fix that at some point, but probably not until #169.)

@timholy
Copy link
Owner

timholy commented Dec 14, 2021

Now there are tests!

@timholy
Copy link
Owner

timholy commented Dec 14, 2021

Anything here likely to be breaking? Otherwise 1.0 is ready to go.

@IanButterworth
Copy link
Collaborator Author

I don't think so. It just needs code branches depending on julia version >= 1.8 (or detecting if profile metadata is present, perhaps).
I could try to get this done over the weekend, if that's ok?

@timholy
Copy link
Owner

timholy commented Dec 14, 2021

If it's non-breaking, there's no urgency. If it is, I'll wait. I'm pretty happy using master for a while.

So, 🤷 Just let me know what you want me to do, and don't distort your schedule to rush this out. I'll be starting holiday travel around the weekend anyway.

@IanButterworth
Copy link
Collaborator Author

I think it can be done without breaking, so don't let this stop 1.0

@timholy
Copy link
Owner

timholy commented Dec 14, 2021

OK, I'll get it out. That will shake out any remaining bugs in GtkObservables/Gtk. The landmark change was a whopping two lines: JuliaGraphics/Gtk.jl#600. The rest has been finishing WIP and merging it. But of course wider usage often turns up other issues, and I might as well see what that shakes out.

@IanButterworth
Copy link
Collaborator Author

(just a rebase at this stage)

@IanButterworth
Copy link
Collaborator Author

IanButterworth commented Dec 23, 2021

@timholy what do you think the save/open buttons should do once thread paging is added? I'm thinking it should save/open all pages?

@IanButterworth
Copy link
Collaborator Author

IanButterworth commented Jan 6, 2022

Ok, this is passing locally for me with timholy/FlameGraphs.jl#44 now

I originally wanted to make the load/save buttons work on all pages at once, but figuring that out was stalling this.. perhaps that could be a future PR?

With this PR each page has its own load/save buttons.

For julia <1.8 or if metadata is removed, a single tab shows labeled as "all threads" which I think is informative so worth keeping.

@IanButterworth IanButterworth marked this pull request as ready for review January 6, 2022 02:09
@IanButterworth IanButterworth changed the title WIP: Add paging by thread Add paging by thread Jan 6, 2022
@IanButterworth
Copy link
Collaborator Author

When testing this I suggest using this workaround JuliaLang/julia#35552 (comment)

@IanButterworth IanButterworth changed the title Add paging by thread Add paging by thread (and task, optionally) Jan 6, 2022
@codecov
Copy link

codecov bot commented Jan 11, 2022

Codecov Report

Merging #176 (55f2857) into master (1e91348) will decrease coverage by 1.10%.
The diff coverage is 81.57%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #176      +/-   ##
==========================================
- Coverage   80.92%   79.82%   -1.11%     
==========================================
  Files           2        2              
  Lines         173      228      +55     
==========================================
+ Hits          140      182      +42     
- Misses         33       46      +13     
Impacted Files Coverage Δ
src/precompile.jl 0.00% <0.00%> (ø)
src/ProfileView.jl 87.08% <83.78%> (-3.25%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1e91348...55f2857. Read the comment docs.

@IanButterworth
Copy link
Collaborator Author

I think this is ready. I'll merge and release 1.1.0

@IanButterworth IanButterworth merged commit 073af90 into timholy:master Jan 14, 2022
@IanButterworth IanButterworth deleted the ib/threads branch January 14, 2022 19:19
@timholy
Copy link
Owner

timholy commented Jan 14, 2022

This looks beautiful! I will be excited to try it out.

@timholy
Copy link
Owner

timholy commented Jan 16, 2022

Looks like we could use a test with expand_tasks = true?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants