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

Proposal: Better progress reporting and cancellation for PortablePdbWriter #2801

Closed
andrewcrawley opened this issue Oct 12, 2022 · 4 comments · Fixed by #2802
Closed

Proposal: Better progress reporting and cancellation for PortablePdbWriter #2801

andrewcrawley opened this issue Oct 12, 2022 · 4 comments · Fixed by #2802
Labels
Enhancement Areas for improvement

Comments

@andrewcrawley
Copy link
Contributor

I'm continuing to work on improving the integration between the decompiler and the Visual Studio debugger. We've found that a surprisingly high number of customers who try the decompiler end up attempting to cancel the decompilation operation, and our current hypothesis is that the lack of progress feedback leads them to think that the operation is "hung", or otherwise unresponsive. Since the decompiler doesn't really support cancellation, this leads to an undesirable situation where the user attempts to cancel the operation in VS, and we're forced to wait for it to complete (probably successfully!) before discarding the results and informing the user that the "cancellation" succeeded.

To improve things, I'm looking at implementing better support for cancellation and progress reporting in PortablePdbWriter, the entry point used by the VS debugger.

I've prototyped the following API, which I believe addresses both of the pain points I mentioned above - better progress reporting so users hopefully won't cancel as often, and better cancellation so they won't have to wait as long when they do cancel.

public static void WritePdb(
    PEFile file,
    CSharpDecompiler decompiler,
    DecompilerSettings settings,
    Stream targetStream,
    bool noLogo = false,
    BlobContentId? pdbId = null,
    CancellationToken cancellationToken = default(CancellationToken), // New!
    IProgress<WritePortablePdbProgress> progress = null)              // New!

public struct WritePortablePdbProgress
{
    int TotalTypes { get; }
    int TypesWritten { get; }
}

The implementation merely updates the loop over types in WritePdb to check the CancellationToken and send a progress update per iteration.

If you're satisfied with this design, I'd be happy to clean up my prototype and send a PR. We're hoping to be able to get this into the 8.0 release.

@andrewcrawley andrewcrawley added the Enhancement Areas for improvement label Oct 12, 2022
@dgrunwald
Copy link
Member

Cancellation is already supported via the CSharpDecompiler.CancellationToken property.
Adding support for IProgress makes sense.

@siegfriedpammer
Copy link
Member

Should we report progress for each method body being decompiled (in CSharpDecompiler or only per file in WholeProjectDecompiler? Properly displaying the progress in ILSpy would be nice too.

@siegfriedpammer
Copy link
Member

siegfriedpammer commented Oct 12, 2022

For now, I would only add this to WholeProjectDecompiler / PortablePdbWriter on a per-file basis...

@andrewcrawley yes, a PR would be most welcome...

@andrewcrawley
Copy link
Contributor Author

@dgrunwald - Thanks for the info on cancellation, I just submitted changes to the debugger to get that hooked up.

I'll follow up with a PR for the progress reporting, thanks!

@siegfriedpammer siegfriedpammer linked a pull request Oct 21, 2022 that will close this issue
1 task
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jan 20, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Enhancement Areas for improvement
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants