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

Restrict console output for methods and types with QUIET passed #328

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

jashook
Copy link
Contributor

@jashook jashook commented May 20, 2021

In addition, if JitDisasm is set skip methods which do not have the
method name. This avoids compiling all methods when there is only
one that is intended to be dumped.

In addition, if JitDisasm is set skip methods which do not have the
method name. This avoids compiling all methods when there is only
one that is intended to be dumped.
@jashook
Copy link
Contributor Author

jashook commented May 20, 2021

@BruceForstall please route as necessary

@jashook
Copy link
Contributor Author

jashook commented May 27, 2021

/cc @sandreenko

@BruceForstall
Copy link
Member

BruceForstall commented May 27, 2021

also @AndyAyersMS

@BruceForstall
Copy link
Member

The JitDisasm changes worry me, as they're a bit of a hack: they don't parse using the same syntax as the JIT. E.g., they don't handle *. And it doesn't look like the two cases parse the same way.

The verbose changes to DriveAll seem reasonable, but I haven't been able to successfully run DriveAll locally to see what the current behavior is, so I need to investigate that first.

@jashook
Copy link
Contributor Author

jashook commented May 27, 2021

This is correct, anything with the wildcard will currently fail. Adding logic for that, would be a bit annoying but seems required.

@AndyAyersMS
Copy link
Member

I've wanted something like this for a while, though generally I'd prefer to specify the pattern via command line args instead of env vars (which I realize is awkward because pmi's command line parsing is lame).

So maybe (in verbose) mode indicate that the results are filtered?

@jashook
Copy link
Contributor Author

jashook commented May 27, 2021

I am fine with changing this to be passed in via command line arguments. Seems to somewhat diverge from current expectations; however, seems like the correct direction.

As for the filtered indications, sounds like printing method ------ is skipped.

I've wanted something like this for a while

I found pretty quickly it is necessary for large assemblies. Between the disk usage and just the time to dasm a large assembly makes this change a requirement for viewing dasm for a particular method in most assemblies.

@jashook
Copy link
Contributor Author

jashook commented May 27, 2021

Thank you for the initial comments, I will work on the feedback. This is no where near my highest priority, so my guess is I will touch back on this in a week or two.

@BruceForstall
Copy link
Member

fwiw, another workflow could be: superpmi collect a PMI run, then do superpmi replay with asm for a particular function or functions. This would be a one-time hit for the collection, but everything subsequent is really fast. I can give more details if interested.

@jashook
Copy link
Contributor Author

jashook commented May 27, 2021

I think the larger issue here with relying on SuperPMI is the fact that the assemblies are not snapshots. They change often and recollection is expensive and long.

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.

3 participants