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

UI for setting compiled funcs/mods #1888

Merged
merged 11 commits into from May 19, 2021
Merged

UI for setting compiled funcs/mods #1888

merged 11 commits into from May 19, 2021

Conversation

pfitzseb
Copy link
Member

Requires julia-vscode/DebugAdapter.jl#31.
Fixes #1880.

Peek 2021-01-26 22-20

There are still a few issues with the UI and I'd like to add an input because the default search is kinda bad (too fuzzy).

@pfitzseb
Copy link
Member Author

Improved the UI/UX a bunch:
image

This still only works for the attached debugger, but feels pretty good so far. I've also added some better defaults, but haven't played around with them too much.

and allow list of compiled methods to be changed on the fly
@pfitzseb pfitzseb marked this pull request as ready for review January 28, 2021 14:22
@davidanthoff
Copy link
Member

This is amazing!

Should we remove the UI for compile mode in the breakpoints section then?

@pfitzseb
Copy link
Member Author

pfitzseb commented Jan 28, 2021

Hm, we maybe can move the functionality to our own UI, but I'm not as confident in my own sync code compared to the breakpoint handling. Will play around with adding an extra button to toggle compiled mode though.

Oh, and I also don't know how to display this only for Julia debug sessions, but I'm pretty sure there is a way.

@pfitzseb
Copy link
Member Author

Now supports setting functions as interpreted, so we can force all of Base to run in compiled mode except for some functions (I've set a list of default functions here, but not sure how sensible that one is). Requires JuliaDebug/JuliaInterpreter.jl#468.

@pfitzseb
Copy link
Member Author

pfitzseb commented Feb 25, 2021

Alright, this is the workflow for whitelisting certain functions in a compiled module to be interpreted:
Peek 2021-02-25 13-02

I've also added a button for temporarily toggling compiled mode (the red breakpoint looking thing), so we don't abuse the breakpoint UI for that anymore. I think this should be good to go into the next minor in this state.

package.json Show resolved Hide resolved
package.json Show resolved Hide resolved
package.json Show resolved Hide resolved
"Base.",
"-Base.!",
"-Base.all",
"-Base.all!",
Copy link
Contributor

Choose a reason for hiding this comment

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

Are these by defining module, or by namespace module?
i.e. will this also compile my overoads of Base.map(f, ::MyType) etc?

Copy link
Member Author

Choose a reason for hiding this comment

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

These are for all methods of that function, so -Base.map will force interpreted mode for all overloads.

Copy link
Contributor

Choose a reason for hiding this comment

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

We should look into working out how to do it by definitition location, but that can be a follow up PR?

Copy link
Member Author

Choose a reason for hiding this comment

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

Well, this list could be method based instead of function based, but I'm not sure whether that's particularly useful.

package.json Outdated Show resolved Hide resolved
@timholy
Copy link

timholy commented Feb 26, 2021

I do think there's some reason to think seriously about interpreting -Statistics.mean to signify "all methods of Statistics.mean that are owned by Statistics." That said, perhaps it doesn't much matter: if I've written a specialization in a package, it's unlikely I mix that with calls to methods defined in other modules.

Copy link
Member

@davidanthoff davidanthoff left a comment

Choose a reason for hiding this comment

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

Fantastic, just needs a merge conflict resolution.

@davidanthoff davidanthoff added this to Changes requested in v1.2 Mar 12, 2021
@pfitzseb
Copy link
Member Author

Ok, breakpoints now work at least in foreach/map/generators/loops on Julia 1.5. Also fixed an issue with frame cache invalidation.

@pfitzseb pfitzseb moved this from Changes requested to Review missing in v1.2 Mar 15, 2021
@davidanthoff davidanthoff moved this from Review missing to Done in v1.2 Mar 16, 2021
@oppo-source oppo-source removed the request for review from a team April 16, 2021 07:35
Copy link
Member

@davidanthoff davidanthoff left a comment

Choose a reason for hiding this comment

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

Needs merge conflict resolution. Looks to me as if someone updated the submodules on the branch, which is never a good idea :)

@davidanthoff davidanthoff moved this from Done to Changes requested in v1.2 May 12, 2021
@pfitzseb
Copy link
Member Author

Yeah, I checked in submodules because this needed a branch of DebugAdapter to work. Should be all good now :)

@pfitzseb pfitzseb moved this from Changes requested to Review missing in v1.2 May 17, 2021
@davidanthoff davidanthoff moved this from Review missing to Done in v1.2 May 19, 2021
@davidanthoff davidanthoff merged commit 5dbb8e1 into master May 19, 2021
@davidanthoff davidanthoff deleted the sp/compileds branch May 19, 2021 16:33
@davidanthoff davidanthoff moved this from Done to Merged in v1.2 May 19, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
v1.2
Merged
Development

Successfully merging this pull request may close these issues.

Improve debugging performance
4 participants