Add a progress class that can track and report long running operations that happen in LLDB.

[clayborg]

Summary

LLDB can often appear deadlocked to users that use IDEs when it is indexing DWARF, or parsing symbol tables. These long running operations can make a debug session appear to be doing nothing even though a lot of work is going on inside LLDB. This patch adds a public API to install a progress callback that will allow IDEs and other tools to create an activity window that can show users what is going on and keep them informed of expensive operations that are going on inside LLDB.

[clayborg]

So this is a first pass on adding progress notifications to the LLDB public API, and to see how it would be used internally. If we can agree on the implementation, then I will add full tests and documentation to this diff. We used a similar patch to this in an older internal version of lldb-vscode before lldb-vscode was fully open sourced, so we know it works at a base level.

Feel free to comment with suggestions or other issues you see and I will iterate on the design until everyone is happy.

[clayborg]

The current Progress class allows users to specify how many items are to be done when they construct a lldb_private::Progress object, but this part is not used right now for anything yet because it is often hard to know how many items of work there are. For symbol tables in ELF, we have two of them, but we might also create new symbols from EH frame or other sources when there are no symbols, so it was hard to pre-determine a full number of symbols for a given executable. Same goes for mach-o files with the normal symbol table, then new symbols are created from the LC_FUNCTION_STARTS load command that has start addresses for all functions, many of which have symbols. For DWARF, we have 3 ways to index the DWARF: Apple tables, .debug_names, and manually. I will comment inline where we could specify a valid number of work items for DWARF to improve the feedback. The other reason the progress objects don't report immediate feedback is the frequency of the progress callbacks could slow things down if they are called too often with too little work. I am happy to talk about any of these issues if anyone has any questions.

lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp

470–472

Instead of specifying UINT64_MAX as the number of work items here, we can figure out how many compile units exist in the DWARF and use that number, and then a reference to this progress object can be passed into the various indexing calls below and can be updated as the DWARF is parsed.

[shafik]

lldb/include/lldb/Core/Progress.h

34

Maybe I am misunderstanding but if we are going to have a total are we also going to variable to keep track of the expected total number of steps we have in the complete progress?

[clayborg]

lldb/include/lldb/Core/Progress.h

34

Users currently have two options when creating a Progress object:

• accurately specify the "uint64_t total" in the constructor, and then call Progress::Increment() as needed.
• just specify any number (UINT64_MAX is what all current code example do) and don't bother calling Progress::Increment()

Either way the Progress destructor will always report the progress with the value of "m_total" when the destructor is called. This is what all of the current code examples rely on. See my other comments as to why it is tricky to get an accurate total count for the various long running tasks.

[clayborg]

Another thing to clarify: The current design of the Progress objects, when destructed, will report progress with the stored "m_total" value to indicate that the progress is complete. Even if exceptions are thrown, the destructor will still be called. So all current uses of the "Progress" objects, will currently report progress when constructed with a "completed" value of zero:

ProgressCallback(message = "Indexing DWARF for /tmp/a.out", completed = 0, total = UINT64_MAX, baton = <baton>);

Then when the Progress object is destroyed, we will report a completed value that is equal to the "Progress::m_total" to indicate things are done:

ProgressCallback(message = "Indexing DWARF for /tmp/a.out", completed = UINT64_MAX, total = UINT64_MAX, baton = <baton>);

The idea is when the code that installs the progress callback gets its callback called, and if the "completed == total" then the UI can remove the progress indicator for "<message>". The message string is a const string and will be unique and the same pointer value for the message will always be used, so the progress is keyed off of the message pointer value.

[JDevlieghere]

Very cool, this is something I have wanted for a long time. Another really good use case here would be dSYMForUUID which can take such a long time that people think the debugger hangs. I think this will be really powerful on the command line too if we can hook this up with a little progress bar that appears and disappears during long running tasks. I left a few comments inline.

lldb/include/lldb/Core/Progress.h

2

I think this belongs more in Utility than Core.

20

I'm not a fan of how we'rere-implementing printf across different utility classes. Can we have this take std::string (that we move into the member) and have the caller use formatv or something to construct the string?

25

Rather than making this a static property of the Progress class, maybe this should be a member of the debugger? And then you could have a factory in the debugger CreateProgress that hands out a new Progress instance. The reason I'd like to have this tied to the debugger is that I imagine using this to show a little progress bar on the command line. When we do that we can have the debugger wrap its own callback around the one provided by the user instead of having to hijack it. It would also align with where the function is exposed in the SB API.

31

What do you imagine this baton to be used for?

33

Why not uint64_t?

[clayborg]

lldb/include/lldb/Core/Progress.h

2

I can move this. It used to rely on stuff in Debugger.h/Debugger.cpp as I was iterating on it, but I removed that requirement, so yes, utility would work fine.

20

Were are not implementing printf, just forwarding to a var args. If we switch over to anything we should switch to ConstString since we report the same ConstString on each callback.

25

I too wanted this, but Module and ObjectFile stuff isn't tied to a specific debugger since all modules within one LLDB process shared the same global module list. So at all of the current call sites where we report progress, there is no access to a debugger since they aren't unique to one.

So given that we have no access to a debugger at any of the current progress call sites, we can't use a CreateProgress factory, and it removes the need for this to be part of the debugger IMHO. I did start off with this being in Debugger.h and Debugger.cpp, but since it ends up just calling a callback, it isn't needed if we can't actually have progress dialogs that are tied to a specific debugger. So we expose it as part of the debugger at the SB API level, but it isn't tied to it internally.

One idea is that the progress dialogs _could_ include a "debugger_id" field that is optionally set, but since the main pain points: DWARF indexing, symbol table parsing, and symbol fetching isn't tied to a debugger, it seems like overkill.

Let me know what your thoughts are.

31

It is common for any callback to also have a baton in case you have a class that was instantiated for the progress reporting, you can specify this the baton as being the class pointer so you can re-use it. We have this type of thing in all other callbacks.

33

Yes, will fix!

[clayborg]

lldb/include/lldb/Core/Progress.h

20

If we do switch to "ConstString message" can you add inline code suggestions for using formatv for one of the Progress constructors to see how it would look?

[jingham]

I haven't thought about the details of this implementation closely yet. How will this handled nested progress bars? For instance, if I'm Indexing (maybe even on multiple threads) and then one of the threads results in a debug symbols fetch request (e.g. dsymForUUID). Is the consumer just supposed to check the string that comes into the progress callback to match the now two simultaneous progress elements?

[clayborg]

In D97739#2596218, @jingham wrote:

I haven't thought about the details of this implementation closely yet. How will this handled nested progress bars? For instance, if I'm Indexing (maybe even on multiple threads) and then one of the threads results in a debug symbols fetch request (e.g. dsymForUUID). Is the consumer just supposed to check the string that comes into the progress callback to match the now two simultaneous progress elements?

Nesting doesn't matter. I am in the process of adding a "uint64_t progress_id;" to each progress class that will be included in the callback. This will allow as many progress activities to be created from as many threads as needed. All activities will show up as top level activities. So the user will use the "uint64_t progress_id" as the unique indicator. This allows the same message to be used for multiple different progress indicators to avoid potential conflict if the user uses too simple of a string. I am about to post an update. Check the Progress.h header doc once I update this patch, hopefully this will answer any questions you might have.

[clayborg]

Added a "uint64_t progress_id;" as a member variable of the lldb_private::Progress class to help users track individual progress dialogs without conflict. Prior to this the message was assumed to be unique.

Added code to ensure that we only report one update there "completed == total". This allows clients that install the callbacks to ensure they will only receive one completion event.

Added code the ensure we don't increment past the total amount and also can't run into unsigned overflow conditions that could have resulted in sending an update where the new "completed" amount was less than before.

Added header doc to explain the usage of the progress class.

[clayborg]

Forgot to mention I added a lldb-vscode implementation after someone told me they now support progress reporting in Visual Studio Code. See https://microsoft.github.io/debug-adapter-protocol/specification#Events_ProgressStart

[clayborg]

Added a more granular progress when manaully indexing the DWARF where we report on unit of progress for parsing all dies in a unit, and one unit of progress for indexing each compile unit, and one unit of work when merging the IndexSets.

Improved the filename reported to use the module's description so that the progress shows archive names + object file ("libfoo.a(bar.o)") instead of just the archive name.

[clayborg]

This is fully hooked up and working in Visual Studio Code now! Works well. The Visual Studio Code shows any progress that takes more than 0.5 seconds down in the bottom toolbar for me. We can and should discuss these changes now that they are working.

[teemperor]

Thanks for working on this. I really like the idea and I think this would be really cool to have. I do have some concerns about the way this is implemented though (see inline comments).

lldb/include/lldb/Core/Progress.h

20

I don't see why we even need formatv for the current examples which just concatenate two strings. I also don't see why the constructor needs to do any formatting in the first place.

FWIW, with formatv this could be done like this:

Progress bla(3423, llvm::formatv("Indexing file {0}", path))

Or, as this is also just concatenating two strings:

Progress bla(3423, "Indexing file " + path)

What's even better about this though is that we can move the total parameter to the end where we can just make it optional (and even better, a real llvm::Optional that defaults to llvm::None instead of a magic UINT64_MAX value).

100

std::string

lldb/source/Core/Module.cpp

1076

I want to ask why but I fear the answer.

lldb/source/Core/Progress.cpp

18

I would really like to avoid the global state that is introduced here, but I assume the idea here is to avoid having to pass around a Debugger object to all parts of LLDB (especially the ones which aren't tied to a Debugger).

Anyway, a few comments on the current approach:

• One callback is rather limiting. If I have an IDE with a progress bar that has an embedded LLDB command line, then I guess both want to subscribe to progress events? Right now the one that starts later will steal the progress notifications from whatever client has subscribed earlier.
• This needs to be made thread-safe (right now I can call lldb::SBDebugger::SetProgressCallback and just snitch away g_callback in the middle of printing the progress -> crash).

As we probably end up with some kind of global state object that manages this stuff, I would also suggest we put this into the normal subsystem init/deinit infrastructure (i.e., give it a Initialize/Terminate call). This way this won't be another "intentional leak" and we can properly use this in unit tests (where we can init/deinit subsystems and cleanup this state automatically).

39

So, just to be clear what's going on here: This is the function we expect several 'working' threads to call? From the current uses in ManualDWARFIndex that seems to be the case, but I don't see how this will work out.

1. Whoever receives the "progress report" callback needs to be ready to handle receiving multiple events from multiple threads. The VSCode callback isn't thread safe from what I can see and user-callbacks will also not be thread-safe. Either we make it really, really, really clear that the progress callbacks arrive from several threads or we somehow try to make this code handle the multi-threading for the user.
2. The logic here isn't thread-safe (m_completed is atomic, but that doesn't make the surrounding logic thread-safe).

[clayborg]

lldb/include/lldb/Core/Progress.h

100

Agreed, after I added the "m_id" member variable, the ConstString value is no longer relied upon to unique the progress.

lldb/source/Core/Module.cpp

1075–1076

This is the main cause for deadlocks with DWARF logging. We have spoken about this on the list before where we should drop the mutex from this call to avoid deadlocking logging calls, but that also applies to this case where Progress objects might be constructed on any thread. The m_arch, m_file and m_object variables are setup very early in the Module object's lifetime, so it should be fine to not have to lock this mutex. I can submit this as a separate patch so we can discuss, but I included it here to avoid deadlocks.

lldb/source/Core/Progress.cpp

18

Yeah, I started off putting stuff in the Debugger as static calls, but then it really had nothing to do with the debugger since we don't ever have a debugger to pass around, nor does it make sense since the module list is global and all debuggers can use it and this is where the bulk of the long running operations happen.

I can add multiple callback support and thread safety. The only issue I see with multiple callbacks is you would need an extra call to say "remove this callback + baton from the list" if we ever want to remove the callback. Currently you can set the callback to null.

39

Yes multiple threads can call this.

1. The callback function must be made thread safe as these callbacks will come from one or more threads. I will add logic to make the callback thread safe in lldb-vscode.cpp.

2. You are correct, I can add a mutex to allow the math that I recently added to be thread safe and switch m_completed to not be atomic.

[clayborg]

Update fixes:

• make progress callback and baton members of a lldb_private::Debugger object to allow each debugger to have their own callbacks.
• switch to have std::string title and clients use llvm::formatv() to create the title to avoid printf variadic args in progress constructor
• make the progress class thread safe using a mutex
• verified that lldb-vscode callback is threadsafe

[clayborg]

This version should address all comments and issue that I am aware of. Now the callbacks are registered with each debugger and the Progress class no longer has any global state. The Progress class calls a static method on the debugger which allows the Debugger class to iterate over the global debugger list and report progress to any debuggers that have callbacks. This also means that later we can modify the Progress class to allow debugger specific progress notifications by modifying the Progress class to take a debugger ID or pointer as an optional constructor argument and then that progress can be reported to only that debugger instance. I don't have any places I need this yet, so I haven't added support for it.

lldb/include/lldb/Core/Progress.h

107

Using llvm::Optional was too messy. I tried, but since the callback requires a completed amount and total to be supplied at all times, it was much easier and cleaner to use UINT64_MAX since that would need to be sent anyway.

[clayborg]

Fix typo in headerdoc comment.

[wallace]

This looks really nice. The only thought I have is that you should accept multiple callbacks instead of only one, as the current implementation discards any existing callback if SetProgressCallback is called twice. I know that it'd be rare that two or more callbacks are registered simultaneously, but it might happen in the future and you could save someone's time by making this a little bit more flexible.

[clayborg]

So we have had a vote for allowing multiple callbacks. Any other main issues with the approach in this patch? If everyone is mostly happy, please chime in and I will make unit tests and vscode tests.

[wallace]

I'm quite happy with it besides the multiple callback thing.

[jingham]

This way of doing progress is going to look odd in anything that uses multiple debuggers. If I'm reading the code aright, if I have two debuggers, and a target in Debugger A starts doing something that would cause progress traffic, both debuggers will show activity. Given that the candidate for progress that is most common is down in SymbolFile, it's going to be hard to fix that. You'd have to have some way of marking the API boundary from "where you have a Target" to "where you don't have a Target" and record (maybe with TSD) that this target is using this API on this thread... Then you could use that to send the progress to the right actor.

Note, this is the way Xcode runs lldb, so this isn't a trivial concern. OTOH, it might be more trouble than it's worth to fix. But we should make clear that each callback will receive events caused by any of the extant debuggers, regardless of which debugger it was registered for. That very much changes how you would present the results.

I also wonder a little bit why you went with a callback rather than sending progress events? You wouldn't have had to invent a callback mechanism, then. Whoever cared about progress events would sign up for the "ProgressEvent" bit on their Debugger, and could do what they like.

[clayborg]

In D97739#2607869, @jingham wrote:

This way of doing progress is going to look odd in anything that uses multiple debuggers. If I'm reading the code aright, if I have two debuggers, and a target in Debugger A starts doing something that would cause progress traffic, both debuggers will show activity.

that is true, but it is a global module repository that benefits both debuggers. And I very rarely debug two things at the same time, so most of the time for most people this will be beneficial and shouldn't cause too much confusion.

We can also add debugger specific progress in the future when we do have a long running task that can be associated with a specific debugger.

Given that the candidate for progress that is most common is down in SymbolFile, it's going to be hard to fix that. You'd have to have some way of marking the API boundary from "where you have a Target" to "where you don't have a Target" and record (maybe with TSD) that this target is using this API on this thread... Then you could use that to send the progress to the right actor.

Yeah, we would need to try and catch all of the areas that end up making queries just to avoid broadcasting extra progress to multiple debuggers. But again, 99.9% of the time people are debugging one thing.

Note, this is the way Xcode runs lldb, so this isn't a trivial concern. OTOH, it might be more trouble than it's worth to fix. But we should make clear that each callback will receive events caused by any of the extant debuggers, regardless of which debugger it was registered for. That very much changes how you would present the results.

It isn't too often that people debug multiple things at the same time. It does happen. But it would be ok for the activity in each window to show what is going on for things that can't be tied back to a specific debugger since it will benefit them all. Kind of like the old Apple mail activity view if anyone remembers that, it would be find for each window to show the same thing.

This at least allows us to supply feedback for long running operations where clients right now have no way of knowing why the delays are happening and often just kill the debug session when it is taking too long. So some feedback is much better than none IMHO.

I also wonder a little bit why you went with a callback rather than sending progress events? You wouldn't have had to invent a callback mechanism, then. Whoever cared about progress events would sign up for the "ProgressEvent" bit on their Debugger, and could do what they like.

We can go with SBBroadcaster and SBListener events if that is what everyone thinks is the best way forward. SBDebugger currently doesn't vend any events so that would need to be added. SBDebugger but does use a logging callback which probably could be also converted to SBEvents.

[clayborg]

SBDebugger currently doesn't vend any events so that would need to be added.

I should clarify this: SBDebugger doesn't have any SBDebugger specific SBEvents that it vends or that people can sign up for. It does vend events for the targets / processes that it owns, just no SBDebugger specific events.

Anyone else have any opinion on SBEvent versus a callback? I like the SBEvent idea because this allows API access to this which means this can be tested easily in the python API tests.

[clayborg]

One serious vote against the SBEvent way of doing things is that it might stop progress notification from appearing in the UI immediately if someone is currently calling something that causes a long running operation from the main thread that is running the SBEvent loop...

[wallace]

I think that the mechanism used to show progress should be lock-free and mostly unblocked, as it's merely an observer without side effects within LLDB. So from the last thing you said it seems that SBEvent doesn't like a good solution.

[clayborg]

I agree that we should avoid SBEvent after thinking about it.

[jingham]

In D97739#2607993, @clayborg wrote:

I agree that we should avoid SBEvent after thinking about it.

First off, doing long running operations on a thread that is the one handling the major lldb events is obviously a bad idea. The driver doesn't do it this way. Commands get run on the main thread, and events get processed on the event-handler thread.

Secondly, there's no reason that the listener for progress events has to call "WaitForEvents" along with all the other events the Debugger might be waiting for. You could just set up a listener thread for all these events (they would have their own event bit) if you wanted somebody just monitoring progress events.

It also has the advantage that callbacks can't stall the progress of lldb, since the event just gets fired off, and then you go on your way...

[clayborg]

In D97739#2608066, @jingham wrote:

In D97739#2607993, @clayborg wrote:

I agree that we should avoid SBEvent after thinking about it.

First off, doing long running operations on a thread that is the one handling the major lldb events is obviously a bad idea. The driver doesn't do it this way. Commands get run on the main thread, and events get processed on the event-handler thread.

True, but the diver isn't a great example of using the API like an IDE would. And anything can kick off expensive events in the debugger. Xcode, as you know, as a thread that listens for events and does process control. This process control thread is responsible for many things including fetching the frame variables when needed. So just the act of having a dynamic type that needs to lookup a class by name could cause this expensive event to trigger. And with many other APIs, you never know what might trigger some expensive lookup to happen. Just the act of consuming a process stopped event on the process control thread is enough to trigger a python script or other LLDB commands in the event handler itself that could cause the expensive event to trigger and cause long delays. So there are plenty of reasons that this could cause delays.

Secondly, there's no reason that the listener for progress events has to call "WaitForEvents" along with all the process events it's waiting for on the debugger. You could just set up a listener thread for all these events (they would have their own event bit) if you wanted somebody just monitoring progress events.

That is true. But spinning up a thread just to listen for progress events seems like a bit of overkill, but it can easily be done.

It also has the advantage that callbacks can't stall the progress of lldb, since the event just gets fired off, and then you go on your way...

Using events does also has the advantage being able to receive all progress events on the same thread. Though since most GUI programs can only draw to the window server on the main thread, they will still need to setup things to forward these events to the main thread. Same goes for the callback methods though.

I am happy to try the SBEvent approach if anyone else chimes in

[friss]

In D97739#2607961, @clayborg wrote:

In D97739#2607869, @jingham wrote:

This way of doing progress is going to look odd in anything that uses multiple debuggers. If I'm reading the code aright, if I have two debuggers, and a target in Debugger A starts doing something that would cause progress traffic, both debuggers will show activity.

that is true, but it is a global module repository that benefits both debuggers. And I very rarely debug two things at the same time, so most of the time for most people this will be beneficial and shouldn't cause too much confusion.

Just one tidbit here. Most users are actually routinely running tens of debuggers at the same time, because tests run in parallel and they have a debugger attached by default. Now if you have a long running operation kick in in your unit tests, you might already have a different kind of issue, but I’d like to avoid a world where the IDE displays spurious and wrong information because of this.

[clayborg]

In D97739#2608510, @friss wrote:

In D97739#2607961, @clayborg wrote:

In D97739#2607869, @jingham wrote:

This way of doing progress is going to look odd in anything that uses multiple debuggers. If I'm reading the code aright, if I have two debuggers, and a target in Debugger A starts doing something that would cause progress traffic, both debuggers will show activity.

that is true, but it is a global module repository that benefits both debuggers. And I very rarely debug two things at the same time, so most of the time for most people this will be beneficial and shouldn't cause too much confusion.

Just one tidbit here. Most users are actually routinely running tens of debuggers at the same time, because tests run in parallel and they have a debugger attached by default. Now if you have a long running operation kick in in your unit tests, you might already have a different kind of issue, but I’d like to avoid a world where the IDE displays spurious and wrong information because of this.

But you wouldn't actually hookup any progress callbacks on any of these debuggers right? You don't make a window for each test, IIRC you are only running it under the debugger so you can report issues by using the debugger. Am I remember this correctly? What happens when a test crashes? If you are running 100 tests in parallel and 10 of them crash, do you open a window for each one that does crash? Or do you manually have to debug the test in order to stop at breakpoints or at a crash?

If we truly need debugger specific task progress, that is a lot more work and we have no great solution. One idea is we could end up having progress items take a SymbolContextScope pointer as an optional parameter that would allow us to grab the module pointer from it and then ask the debugger if any targets contain this module prior to reporting progress. This would be a bit expensive code for a lot of quick progress updates as we would need to iterate over each target and each target's module list and we would still have many system libraries reporting progress to all debuggers when targets contain the same system libraries.

The threading overhead and expense of delivering SBEvents also seems like overkill as threading itself and waiting using a mutex + condition will slow down progress delivery especially if we have a progress that does a bunch of updates. And once we receive the event we will need to make a static function call to extract all progress variables (total, completed, progress_id, baton, etc).

In D97739#2608510, @friss wrote:

In D97739#2607961, @clayborg wrote:

In D97739#2607869, @jingham wrote:

This way of doing progress is going to look odd in anything that uses multiple debuggers. If I'm reading the code aright, if I have two debuggers, and a target in Debugger A starts doing something that would cause progress traffic, both debuggers will show activity.

that is true, but it is a global module repository that benefits both debuggers. And I very rarely debug two things at the same time, so most of the time for most people this will be beneficial and shouldn't cause too much confusion.

Just one tidbit here. Most users are actually routinely running tens of debuggers at the same time, because tests run in parallel and they have a debugger attached by default. Now if you have a long running operation kick in in your unit tests, you might already have a different kind of issue, but I’d like to avoid a world where the IDE displays spurious and wrong information because of this.

[jingham]

In D97739#2610521, @clayborg wrote:

In D97739#2608510, @friss wrote:

In D97739#2607961, @clayborg wrote:

In D97739#2607869, @jingham wrote:

This way of doing progress is going to look odd in anything that uses multiple debuggers. If I'm reading the code aright, if I have two debuggers, and a target in Debugger A starts doing something that would cause progress traffic, both debuggers will show activity.

that is true, but it is a global module repository that benefits both debuggers. And I very rarely debug two things at the same time, so most of the time for most people this will be beneficial and shouldn't cause too much confusion.

Just one tidbit here. Most users are actually routinely running tens of debuggers at the same time, because tests run in parallel and they have a debugger attached by default. Now if you have a long running operation kick in in your unit tests, you might already have a different kind of issue, but I’d like to avoid a world where the IDE displays spurious and wrong information because of this.

But you wouldn't actually hookup any progress callbacks on any of these debuggers right? You don't make a window for each test, IIRC you are only running it under the debugger so you can report issues by using the debugger. Am I remember this correctly? What happens when a test crashes? If you are running 100 tests in parallel and 10 of them crash, do you open a window for each one that does crash? Or do you manually have to debug the test in order to stop at breakpoints or at a crash?

If we truly need debugger specific task progress, that is a lot more work and we have no great solution. One idea is we could end up having progress items take a SymbolContextScope pointer as an optional parameter that would allow us to grab the module pointer from it and then ask the debugger if any targets contain this module prior to reporting progress. This would be a bit expensive code for a lot of quick progress updates as we would need to iterate over each target and each target's module list and we would still have many system libraries reporting progress to all debuggers when targets contain the same system libraries.

The threading overhead and expense of delivering SBEvents also seems like overkill as threading itself and waiting using a mutex + condition will slow down progress delivery especially if we have a progress that does a bunch of updates. And once we receive the event we will need to make a static function call to extract all progress variables (total, completed, progress_id, baton, etc).

I don't want to push this too hard, either way will work. But note that one of the big advantages of the event version is that we aren't running arbitrary code fairly deep in the Symbol side of lldb. At the cost of taking a lock to deliver the event, we get all this code out of the depth of the symbol reader and into a separate reporter thread.

I think the important thing here is that the code that detects the progress event not stall the Symbol processing. But I don't see the need to be super-performant in reporting the event. If the progress events are going by so fast that taking a lock to get the event and report on it matters, then the progress was going fast enough that the user probably won't care about it.

Another nice thing about the event version is that it allows you to coalesce reports if they come in too quickly, since you can peek at the event queue when you get the first event, and pull any other pending ones off. So for instance if you get a "started event" and peeking at the queue find that you also already have the completed event, you could choose not to report progress, or just completed...

In D97739#2608510, @friss wrote:

In D97739#2607961, @clayborg wrote:

In D97739#2607869, @jingham wrote:

This way of doing progress is going to look odd in anything that uses multiple debuggers. If I'm reading the code aright, if I have two debuggers, and a target in Debugger A starts doing something that would cause progress traffic, both debuggers will show activity.

that is true, but it is a global module repository that benefits both debuggers. And I very rarely debug two things at the same time, so most of the time for most people this will be beneficial and shouldn't cause too much confusion.

Just one tidbit here. Most users are actually routinely running tens of debuggers at the same time, because tests run in parallel and they have a debugger attached by default. Now if you have a long running operation kick in in your unit tests, you might already have a different kind of issue, but I’d like to avoid a world where the IDE displays spurious and wrong information because of this.

[clayborg]

Added support for getting progress events via SBEvent delivery.

[jingham]

Thanks for doing this! The event version looks pretty clean to me. I think we should go that way. I don't think we should have two ways, that seems confusing and still leaves us calling unknown user code in the middle of symbol updates...

[clayborg]

In D97739#2636343, @jingham wrote:

Thanks for doing this! The event version looks pretty clean to me. I think we should go that way. I don't think we should have two ways, that seems confusing and still leaves us calling unknown user code in the middle of symbol updates...

Sounds good, just wanted to make sure this was the way we want to go. I will remove the callback stuff.

[clayborg]

Removed all callback related code and now only use events.

[clayborg]

lldb/include/lldb/API/SBDebugger.h

44

@jingham: do we need this GetBroadcasterClass()? Or would this only be used to listen to all debugger events as new debuggers are created? Also see my question later that asks if we need the broadcast manager in the Broadcaster we are using in the Debugger.h/Debugger.cpp.

lldb/source/Core/Debugger.cpp

672 ↗

(On Diff #332383)

@jingham: do we need to the m_broadcaster_manager_sp in this broadcaster? Debugger inherits from Broadcaster, but we could change this to "has a" instead of "is a" if we need the broadcast manager.

[jingham]

This seems like the right way to go. I made a couple trivial comment comments.

But also: the feature has the problem that when multiple debuggers are present, there's no way to tell what debugger actually triggered the work, and in that case all Debuggers will get the event. That's not the Progress feature's fault, it's forced on us by the design of the global shared module cache. But we shouldn't let the fact that that happens be first set of things for which you want to add progress drive the design. Anyway, it seems to me like it would be simple to set this up to support other areas where there is a debugger, and doing that up front would keep people from accidentally adding progress but not providing the debugger because the API makes it seem like that isn't relevant. I think the only things that you would need to add are:

1. Have Progress::ReportProgress take a Debugger * argument, and only use the static Debugger::ReportProgress when the debugger is NULL. That would make it clear to users of the API that they should provide a Debugger if one is available.
2. Have another bit in the ProgressEventData that tells whether a debugger was supplied or not. That way IDE's that support many debugger's running simultaneously would know to not associate anonymous progress events with any particular debugger, but to put it up in some kind of general progress.

lldb/include/lldb/API/SBDebugger.h

48

remove the "that"

57

It -> If

lldb/include/lldb/Core/Progress.h

67

Not grammatical. Maybe "then an indeterminate progress indicator should be displayed".

lldb/source/API/SBDebugger.cpp

826

These are just drive by reformattings, right? Might be good not to include them in this change.

lldb/source/Core/Debugger.cpp

1187 ↗

(On Diff #332383)

Isn't ReportProgressPrivate the way that any part of the system that DOES have access to a Debugger should call to update Progress? If so I don't think it's right to call this API "Private". This should be documented as the preferred method, and the static ReportProgress should be documented as only to be used in situations where you can't find your way to a debugger.

lldb/source/Core/Progress.cpp

50

IMO it would be better for Progress::ReportProgress to take a Debugger *. If the Debugger* is non-null, then we would call Debugger->ReportProgressPrivate or whatever is the appropriate name, and only if the Debugger is NULL would we call the static function. It would be better if we have the right way to do things from the get-go, and since the Debugger is delivering the event, that's pretty clearly to call ReportProgress for the relevant Debugger.

lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp

1897–1899

Also an unrelated reformatting...

[jingham]

lldb/include/lldb/API/SBDebugger.h

44

It is needed by the mechanism for waiting for the events from all broadcasters of a given type. Not sure why you wouldn't want to do this for Debugger events.

GetBroadcasterClass is used in the Debugger DefaultEventHandler to separate the treatment for the various event sources the Debugger waits for. One of the missing pieces of this patch is to get the Driver's default event handling to do something with progress updates. I presume that would go through the same DefaultEventHandler, and you'd use the comparison of GetStaticBroadcasterClass and GetBroadcasterClass in the same way that code does for all the other event classes.

lldb/source/Core/Debugger.cpp

672 ↗

(On Diff #332383)

Given that at least in the first pass all the events are not specific to a debugger, in a Multi-Debugger UI I could very well imagine that instead of having any individual debugger's Listener listen for these events, you would make some thread listen for them all, and then put up some System Wide progress UI. That would make it much less confusing than seeming to associate events with debuggers you don't know they pertain to.

And even when we start reporting Progress that was associated with a Debugger, we will still have all these unassociated events from the module cache. So you could still imagine having a listener for all debuggers, that only reports progress that has no debugger associated with it, and then have individual Debugger's report progress pertaining directly to them.

So I do think we need to have a BroadcasterManager that allows you to sign up for all Debugger events from any debugger that will be created.

[clayborg]

Added the option for people to add a debugger to a lldb_private::Progress instance. This allows progress to be reported only to specific debuggers.

[clayborg]

lldb/source/API/SBDebugger.cpp

826

I submit with "arc diff" and it will cause lint errors if I don't allow it to fix the lint errors it finds.

lldb/source/Core/Debugger.cpp

1188 ↗

(On Diff #332485)

In the Progress class I currently only store a pointer to the debugger if one was specified. This means there is a lifetime issue that can arise. If I do the callback for Debugger::ReportProgress(...) above, then we will only notify debugger instances that are still around and in the global list. If not then we need to either store a DebuggerSP or DebuggerWP in the Progress object, which we can do if truly needed, but I don't like DebuggerSP as I don't want a Progress to keep a debugger around. DebuggerWP is possible if we really need this. Let me know what you think. Debugger::ReportProgressPrivate() could be make into a static function if needed so that no one sees it from Debugger.h.

lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp

1897–1900

again, I use "arc diff" which must fix lint issues.

[jingham]

lldb/source/API/SBDebugger.cpp

826

I'm 100% not in favor of tools that force irrelevant changes to be included. But that is a suggested tool so somebody must like that.

lldb/source/Core/Debugger.cpp

1188 ↗

(On Diff #332485)

I think it's fine to drop progress notifications on the floor if they were debugger specific and that debugger is no longer around. I don't think it makes sense for the Progress to keep Debugger alive. As long as you check on the existence as you do here, you won't end up calling into a bad debugger, so the way you've done it looks fine. You might want to have an:

if (is_debugger_specific) break;

After the call to ReportProgressPrivate. It's unlikely we'll ever have enough debugger's that this a performance issue, but still looks weird to keep checking after we found the one we want.

BTW, it might be better to have the Progress store the DebuggerID rather than the pointer. It would make it clear we aren't making claims about keeping the debugger alive.. That would simplify this code, since you could:

if (debugger_id != LLDB_INVALID_DEBUGGER_ID) {
 DebuggerSP debugger_sp = Debugger::FindDebuggerWithID(debugger_id)
 if (debugger_sp) debugger_sp->ReportProgressPrivate(..., true);
 return;
}

Then your original loop just calling ReportProgressPrivate on all the debuggers, passing false for "debugger_specific".

[dblaikie]

lldb/source/API/SBDebugger.cpp

826

They aren't forced - you can submit with linter errors if they don't seem helpful.

Pre-committing format changes in standalone NFC commits would generally be preferable. & the linter shouldn't be flagging untouched lines - is it?

[clayborg]

lldb/source/API/SBDebugger.cpp

826

Actually come to thing about it, it is probably because my editor removes trailing newlines and then the linter deems those lines fair game...

lldb/source/Core/Debugger.cpp

1188 ↗

(On Diff #332485)

I like the debugger ID storage idea. I will do that, and break out once we find the right debugger.

[clayborg]

Don't store a debugger pointer in the lldb_private::Progress class, store a debugger ID as an optional value. Then use this value in the debugger report progress function to do the right thing.

[jingham]

This looks good to me.

lldb/source/API/SBDebugger.cpp

826

It looks like the file had changes 700 lines before & 900 lines after, but these lines weren't changed except by the linter...

lldb/source/Core/Debugger.cpp

1188 ↗

(On Diff #332485)

BTW, I made up LLDB_INVALID_DEBUGGER_ID, apparently...

[dblaikie]

lldb/source/API/SBDebugger.cpp

826

Yeah, if your editor is touching otherwise unmodified lines that'll be a problem (for the linter, or without the linter - you'd end up with unrelated changes in the diff, etc).

clang-format can be integrated into some editors (vim, for instance) to auto-format only changed lines on save, which is what I use - not sure if there's an option to use that in your workflow.

[labath]

This test is flaky (even after the vdso fix by raphael), and it seems like it could be a genuine problem: http://lab.llvm.org:8011/#/builders/68/builds/9440, http://lab.llvm.org:8011/#/builders/68/builds/9409, http://lab.llvm.org:8011/#/builders/68/builds/9403, http://lab.llvm.org:8011/#/builders/68/builds/9383, ...

[clayborg]

I will take a look and apply this patch on my linux server and enable ASAN. Looks like lldb-vscode must be crashing from what the errors look like.

[clayborg]

I built on mac with ASAN and ran the tests and they passed just fine. I was also able to compile on Debian linux and the tests ran ok for me. Can anyone else reproduce this failure?

lldb/include/lldb/API/SBDebugger.h

 //===-- SBDebugger.h --------------------------------------------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLDB_API_SBDEBUGGER_H
 #define LLDB_API_SBDEBUGGER_H
 
 #include <stdio.h>
 
 #include "lldb/API/SBDefines.h"
 #include "lldb/API/SBPlatform.h"
 
 namespace lldb {
 
 class LLDB_API SBInputReader {
 public:
   SBInputReader() = default;
   ~SBInputReader() = default;
public:
   SBDebugger();
 
   SBDebugger(const lldb::SBDebugger &rhs);
 
   SBDebugger(const lldb::DebuggerSP &debugger_sp);
 
   ~SBDebugger();
 
   lldb::SBDebugger &operator=(const lldb::SBDebugger &rhs);
 
   static void Initialize();
 
   static lldb::SBError InitializeWithErrorHandling();
 
   static void Terminate();
 
   // Deprecated, use the one that takes a source_init_files bool.
   static lldb::SBDebugger Create();
 
   static lldb::SBDebugger Create(bool source_init_files);
 
   static lldb::SBDebugger Create(bool source_init_files,
                                  lldb::LogOutputCallback log_callback,
                                  void *baton);
 
   static void Destroy(lldb::SBDebugger &debugger);
 
   static void MemoryPressureDetected();
 
+  static void SetProgressCallback(ProgressCallback callback, void *baton);
+
   explicit operator bool() const;
 
   bool IsValid() const;
 
   void Clear();
 
   void SetAsync(bool b);
 

lldb/include/lldb/Core/Progress.h

+//===-- Progress.h ----------------------------------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLDB_CORE_PROGRESS_H
+#define LLDB_CORE_PROGRESS_H
+
+#include "lldb/Utility/ConstString.h"
+#include "lldb/lldb-types.h"
+#include <atomic>
+
+namespace lldb_private {
+
+/// A Progress indicator helper class.
+///
+/// Any potentially long running sections of code in LLDB should report
+/// progress so that clients are aware of delays that might appear during
+/// debugging. Delays commonly include indexing debug information, parsing
+/// symbol tables for object files, downloading symbols from remote
+/// repositories, and many more things.
+///
+/// The Progress class helps make sure that progress is correctly reported
+/// and will always send an initial progress update, updates when
+/// Progress::Increment() is called, and also will make sure that a progress
+/// completed update is reported even if the user doesn't explicitly cause one
+/// to be sent.
+///
+/// The progress is reported via a callback whose type is ProgressCallback:
+///
+///   typedef void (*ProgressCallback)(uint64_t progress_id,
+///                                    const char *message,
+///                                    uint64_t completed,
+///                                    uint64_t total,
+///                                    void *baton);
+///
+/// This callback will always initially be called with "completed" set to zero
+/// and "total" set to the total amount specified in the contructor. This is
+/// considered the progress start event. As Progress::Increment() is called,
+/// the callback will be called as long as the Progress::m_completed has not
+/// yet exceeded the Progress::m_total. When the callback is called with
+/// Progress::m_completed == Progress::m_total, that is considered a progress
+/// completed event. If Progress::m_completed is non-zero and less than
+/// Progress::m_total, then this is considered a progress update event.
+///
+/// This callback will be called in the destructor if Progress::m_completed is
+/// not equal to Progress::m_total with the "completed" set to
+/// Progress::m_total. This ensures we always send a progress completed update
+/// even if the user does not.
+
+class Progress {
+public:
+  /// Construct a progress object that will report information.
+  ///
+  /// The constructor will create a unique progress reporting object and
+  /// immediately send out a progress update by calling the installed callback
+  /// with completed set to zero out of the specified total.
+  ///
+  /// @param [in] total The total units of work to be done. Set this to
+  /// UINT64_MAX if there should be no progress displayed and just show a
+  /// spinning progress indicator.
+  ///
+  /// @param [in] total A printf format string that will be evaluated and will
+  /// becomde the title of this progress activity.
+  Progress(uint64_t total, const char *format, ...)
+      __attribute__((format(printf, 3, 4)));
+
+  /// Destroy the progress object.
+  ///
+  /// If the progress has not yet sent a completion update, the destructor
+  /// will send out a notification where the completed == m_total. This ensures
+  /// that we always send out a progress complete notification.
+  ~Progress();
+
+  /// Increment the progress and send a notification to the intalled callback.
+  ///
+  /// If incrementing ends up exceeding m_total, m_completed will be updated
+  /// to match m_total and no subsequent progress notifications will be sent.
+  ///
+  /// @param [in] amount The amount to increment m_completed by.
+  void Increment(uint64_t amount = 1);
+
+  /// Set the progress notification callback.
+  ///
+  /// If the callback is set to a valid non NULL value, the \a callback
+  /// function will be called when ever a Progress object is constructed,
+  /// Increment is called, or the Progress object is destroyed and has not
+  /// already sent a progress complete notification.
+  static void SetProgressCallback(lldb::ProgressCallback callback, void *baton);
+
+private:
+  void ReportProgress();
+  static lldb::ProgressCallback g_callback;
+  static void *g_callback_baton;
+  static uint64_t g_progress_id;
+  /// The title of the progress activity.
+  ConstString m_message;
+  /// How much work ([0...m_total]) that has been completed.
+  std::atomic<uint64_t> m_completed;
+  /// Total amount of work, UINT64_MAX for non deterministic progress.
+  const uint64_t m_total;
+  /// A unique integer identifier for progress reporting.
+  const uint64_t m_id;
+  /// Set to true when progress has been reported where m_completed == m_total
+  /// to ensure that we don't send progress updates after progress has
+  /// completed.
+  bool m_complete = false;
+};
+
+} // namespace lldb_private
+
+#endif // LLDB_CORE_PROGRESS_H

lldb/include/lldb/lldb-types.h

 //===-- lldb-types.h --------------------------------------------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLDB_LLDB_TYPES_H
 #define LLDB_LLDB_TYPES_H
 
 #include "lldb/lldb-enumerations.h"
 #include "lldb/lldb-forward.h"
 
 #include <stdint.h>
 
 // All host systems must define:
 //  lldb::thread_t          The native thread type for spawned threads on the
 //  system
 //  lldb::thread_arg_t      The type of the one any only thread creation

 
 namespace lldb {
 typedef void (*LogOutputCallback)(const char *, void *baton);
 typedef bool (*CommandOverrideCallback)(void *baton, const char **argv);
 typedef bool (*CommandOverrideCallbackWithResult)(
     void *baton, const char **argv, lldb_private::CommandReturnObject &result);
 typedef bool (*ExpressionCancelCallback)(ExpressionEvaluationPhase phase,
                                          void *baton);
+typedef void (*ProgressCallback)(uint64_t progress_id, const char *message,
+                                 uint64_t completed, uint64_t total,
+                                 void *baton);
 } // namespace lldb
 
 #define LLDB_INVALID_PROCESS ((lldb::process_t)-1)
 #define LLDB_INVALID_HOST_THREAD ((lldb::thread_t)NULL)
 #define LLDB_INVALID_PIPE ((lldb::pipe_t)-1)
 
 namespace lldb {
 typedef uint64_t addr_t;

lldb/source/API/SBDebugger.cpp

 #include "lldb/API/SBTypeFilter.h"
 #include "lldb/API/SBTypeFormat.h"
 #include "lldb/API/SBTypeNameSpecifier.h"
 #include "lldb/API/SBTypeSummary.h"
 #include "lldb/API/SBTypeSynthetic.h"
 
 #include "lldb/Core/Debugger.h"
 #include "lldb/Core/PluginManager.h"
+#include "lldb/Core/Progress.h"
 #include "lldb/Core/StreamFile.h"
 #include "lldb/Core/StructuredDataImpl.h"
 #include "lldb/DataFormatters/DataVisualization.h"
 #include "lldb/Host/Config.h"
 #include "lldb/Host/XML.h"
 #include "lldb/Initialization/SystemLifetimeManager.h"
 #include "lldb/Interpreter/CommandInterpreter.h"
 #include "lldb/Interpreter/OptionArgParser.h"
void SBDebugger::MemoryPressureDetected() {
   // need to safeguard against this until we can determine what is causing the
   // deadlocks.
 
   const bool mandatory = false;
 
   ModuleList::RemoveOrphanSharedModules(mandatory);
 }
 
+void SBDebugger::SetProgressCallback(ProgressCallback callback, void *baton) {
+  Progress::SetProgressCallback(callback, baton);
+}
+
 bool SBDebugger::IsValid() const {
   LLDB_RECORD_METHOD_CONST_NO_ARGS(bool, SBDebugger, IsValid);
   return this->operator bool();
 }
 SBDebugger::operator bool() const {
   LLDB_RECORD_METHOD_CONST_NO_ARGS(bool, SBDebugger, operator bool);
 
   return m_opaque_sp.get() != nullptr;
SBTarget SBDebugger::CreateTargetWithFileAndArch(const char *filename,
   SBTarget sb_target;
   TargetSP target_sp;
   if (m_opaque_sp) {
     Status error;
     if (arch_cstr == nullptr) {
       // The version of CreateTarget that takes an ArchSpec won't accept an
       // empty ArchSpec, so when the arch hasn't been specified, we need to
       // call the target triple version.
-      error = m_opaque_sp->GetTargetList().CreateTarget(*m_opaque_sp, filename, 
-          arch_cstr, eLoadDependentsYes, nullptr, target_sp);
+      error = m_opaque_sp->GetTargetList().CreateTarget(
+          *m_opaque_sp, filename, arch_cstr, eLoadDependentsYes, nullptr,
+          target_sp);
     } else {
       PlatformSP platform_sp = m_opaque_sp->GetPlatformList()
           .GetSelectedPlatform();
-      ArchSpec arch = Platform::GetAugmentedArchSpec(platform_sp.get(), 
-          arch_cstr);
+      ArchSpec arch =
+          Platform::GetAugmentedArchSpec(platform_sp.get(), arch_cstr);
       if (arch.IsValid())
-        error = m_opaque_sp->GetTargetList().CreateTarget(*m_opaque_sp, filename, 
-            arch, eLoadDependentsYes, platform_sp, target_sp);
+        error = m_opaque_sp->GetTargetList().CreateTarget(
+            *m_opaque_sp, filename, arch, eLoadDependentsYes, platform_sp,
+            target_sp);
       else
         error.SetErrorStringWithFormat("invalid arch_cstr: %s", arch_cstr);
     }
     if (error.Success())
       sb_target.SetSP(target_sp);
   }
-  
   LLDB_LOGF(log,
             "SBDebugger(%p)::CreateTargetWithFileAndArch (filename=\"%s\", "
             "arch=%s) => SBTarget(%p)",
             static_cast<void *>(m_opaque_sp.get()),
             filename ? filename : "<unspecified>",
             arch_cstr ? arch_cstr : "<unspecified>",
             static_cast<void *>(target_sp.get()));
 

lldb/source/Core/CMakeLists.txt

add_lldb_library(lldbCore
   IOHandler.cpp
   IOHandlerCursesGUI.cpp
   Mangled.cpp
   Module.cpp
   ModuleChild.cpp
   ModuleList.cpp
   Opcode.cpp
   PluginManager.cpp
+  Progress.cpp
   RichManglingContext.cpp
   SearchFilter.cpp
   Section.cpp
   SourceManager.cpp
   StreamAsynchronousIO.cpp
   StreamFile.cpp
   UserSettingsController.cpp
   Value.cpp

lldb/source/Core/Module.cpp

if (m_object_name) {
     spec += m_object_name.GetCString();
     spec += ')';
   }
   return spec;
 }
 
 void Module::GetDescription(llvm::raw_ostream &s,
                             lldb::DescriptionLevel level) {
-  std::lock_guard<std::recursive_mutex> guard(m_mutex);
-
   if (level >= eDescriptionLevelFull) {
     if (m_arch.IsValid())
       s << llvm::formatv("({0}) ", m_arch.GetArchitectureName());
   }
 
   if (level == eDescriptionLevelBrief) {
     const char *filename = m_file.GetFilename().GetCString();
     if (filename)

lldb/source/Core/Progress.cpp

+//===-- Progress.cpp ------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "lldb/Core/Progress.h"
+
+#include "lldb/Utility/StreamString.h"
+
+using namespace lldb;
+using namespace lldb_private;
+
+lldb::ProgressCallback Progress::g_callback = nullptr;
+void *Progress::g_callback_baton = nullptr;
+uint64_t Progress::g_progress_id = 0;
+
+Progress::Progress(uint64_t total, const char *format, ...)
+    : m_message(), m_completed(0), m_total(total), m_id(++g_progress_id) {
+  assert(total > 0);
+  StreamString message;
+  va_list args;
+  va_start(args, format);
+  message.PrintfVarArg(format, args);
+  va_end(args);
+  m_message = ConstString(message.GetString());
+  ReportProgress();
+}
+
+Progress::~Progress() {
+  // Make sure to always report progress completed when this object is
+  // destructed so it indicates the progress dialog/activity should go away.
+  m_completed = m_total;
+  ReportProgress();
+}
+
+void Progress::Increment(uint64_t amount) {
+  if (amount > 0) {
+    // Watch out for unsigned overflow and make sure we don't increment too
+    // much and exceed m_total.
+    if (amount > (m_total - m_completed))
+      m_completed = m_total;
+    else
+      m_completed += amount;
+    ReportProgress();
+  }
+}
+
+void Progress::SetProgressCallback(ProgressCallback callback, void *baton) {
+  g_callback = callback;
+  g_callback_baton = baton;
+}
+
+void Progress::ReportProgress() {
+  if (g_callback && !m_complete) {
+    // Make sure we only send one notification that indicates the progress is
+    // complete.
+    m_complete = m_completed == m_total;
+    g_callback(m_id, m_message.GetCString(), m_completed, m_total,
+               g_callback_baton);
+  }
+}

lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp

 #include <algorithm>
 #include <cassert>
 #include <unordered_map>
 
 #include "lldb/Core/FileSpecList.h"
 #include "lldb/Core/Module.h"
 #include "lldb/Core/ModuleSpec.h"
 #include "lldb/Core/PluginManager.h"
+#include "lldb/Core/Progress.h"
 #include "lldb/Core/Section.h"
 #include "lldb/Host/FileSystem.h"
 #include "lldb/Host/LZMA.h"
 #include "lldb/Symbol/DWARFCallFrameInfo.h"
 #include "lldb/Symbol/SymbolContext.h"
 #include "lldb/Target/SectionLoadList.h"
 #include "lldb/Target/Target.h"
 #include "lldb/Utility/ArchSpec.h"
(InfoOr->Segment ? InfoOr->Segment->GetChildren() : *m_sections_up)
         .AddSection(section_sp);
     provider.AddSection(std::move(*InfoOr), std::move(section_sp));
   }
 
   // For eTypeDebugInfo files, the Symbol Vendor will take care of updating the
   // unified section list.
   if (GetType() != eTypeDebugInfo)
     unified_section_list = *m_sections_up;
-  
   // If there's a .gnu_debugdata section, we'll try to read the .symtab that's
   // embedded in there and replace the one in the original object file (if any).
   // If there's none in the orignal object file, we add it to it.
   if (auto gdd_obj_file = GetGnuDebugDataObjectFile()) {
     if (auto gdd_objfile_section_list = gdd_obj_file->GetSectionList()) {
       if (SectionSP symtab_section_sp =
               gdd_objfile_section_list->FindSectionByType(
                   eSectionTypeELFSymbolTable, true)) {
         SectionSP module_section_sp = unified_section_list.FindSectionByType(
             eSectionTypeELFSymbolTable, true);
         if (module_section_sp)
           unified_section_list.ReplaceSection(module_section_sp->GetID(),
                                               symtab_section_sp);
         else
           unified_section_list.AddSection(symtab_section_sp);
       }
     }
-  }  
+  }
 }
 
 std::shared_ptr<ObjectFileELF> ObjectFileELF::GetGnuDebugDataObjectFile() {
   if (m_gnu_debug_data_object_file != nullptr)
     return m_gnu_debug_data_object_file;
 
   SectionSP section =
       GetSectionList()->FindSectionByName(ConstString(".gnu_debugdata"));
m_gnu_debug_data_object_file.reset(new ObjectFileELF(
       GetModule(), gdd_data_buf, 0, &fspec, 0, gdd_data_buf->GetByteSize()));
 
   // This line is essential; otherwise a breakpoint can be set but not hit.
   m_gnu_debug_data_object_file->SetType(ObjectFile::eTypeDebugInfo);
 
   ArchSpec spec = m_gnu_debug_data_object_file->GetArchitecture();
   if (spec && m_gnu_debug_data_object_file->SetModulesArchitecture(spec))
     return m_gnu_debug_data_object_file;
-  
   return nullptr;
 }
 
 // Find the arm/aarch64 mapping symbol character in the given symbol name.
 // Mapping symbols have the form of "$<char>[.<any>]*". Additionally we
 // recognize cases when the mapping symbol prefixed by an arbitrary string
 // because if a symbol prefix added to each symbol in the object file with
 // objcopy then the mapping symbols are also prefixed.
unsigned ObjectFileELF::RelocateDebugSections(const ELFSectionHeader *rel_hdr,
   return 0;
 }
 
 Symtab *ObjectFileELF::GetSymtab() {
   ModuleSP module_sp(GetModule());
   if (!module_sp)
     return nullptr;
 
+  Progress progress(UINT64_MAX, "Parsing symbol table for %s",
+                    m_file.GetFilename().AsCString("<Unknown>"));
+
   // We always want to use the main object file so we (hopefully) only have one
   // cached copy of our symtab, dynamic sections, etc.
   ObjectFile *module_obj_file = module_sp->GetObjectFile();
   if (module_obj_file && module_obj_file != this)
     return module_obj_file->GetSymtab();
 
   if (m_symtab_up == nullptr) {
     SectionList *section_list = module_sp->GetSectionList();

lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp

 //===-- ObjectFileMachO.cpp -----------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "llvm/ADT/StringRef.h"
 
 #include "Plugins/Process/Utility/RegisterContextDarwin_arm.h"
 #include "Plugins/Process/Utility/RegisterContextDarwin_arm64.h"
 #include "Plugins/Process/Utility/RegisterContextDarwin_i386.h"
 #include "Plugins/Process/Utility/RegisterContextDarwin_x86_64.h"
 #include "lldb/Core/Debugger.h"
 #include "lldb/Core/FileSpecList.h"
 #include "lldb/Core/Module.h"
 #include "lldb/Core/ModuleSpec.h"
 #include "lldb/Core/PluginManager.h"
+#include "lldb/Core/Progress.h"
 #include "lldb/Core/Section.h"
 #include "lldb/Core/StreamFile.h"
 #include "lldb/Host/Host.h"
 #include "lldb/Symbol/DWARFCallFrameInfo.h"
 #include "lldb/Symbol/ObjectFile.h"
 #include "lldb/Target/DynamicLoader.h"
 #include "lldb/Target/MemoryRegionInfo.h"
 #include "lldb/Target/Platform.h"
if (n_sect < m_section_infos.size()) {
               section_sp->GetByteSize());
         } else {
           std::string filename = "<unknown>";
           SectionSP first_section_sp(m_section_list->GetSectionAtIndex(0));
           if (first_section_sp)
             filename = first_section_sp->GetObjectFile()->GetFileSpec().GetPath();
 
           Host::SystemLog(Host::eSystemLogError,
-                          "error: unable to find section %d for a symbol in %s, corrupt file?\n",
-                          n_sect, 
-                          filename.c_str());
+                          "error: unable to find section %d for a symbol in "
+                          "%s, corrupt file?\n",
+                          n_sect, filename.c_str());
         }
       }
       if (m_section_infos[n_sect].vm_range.Contains(file_addr)) {
         // Symbol is in section.
         return m_section_infos[n_sect].section_sp;
       } else if (m_section_infos[n_sect].vm_range.GetByteSize() == 0 &&
                  m_section_infos[n_sect].vm_range.GetBaseAddress() ==
                      file_addr) {
         // Symbol is in section with zero size, but has the same start address

 
 size_t ObjectFileMachO::ParseSymtab() {
   LLDB_SCOPED_TIMERF("ObjectFileMachO::ParseSymtab () module = %s",
                      m_file.GetFilename().AsCString(""));
   ModuleSP module_sp(GetModule());
   if (!module_sp)
     return 0;
 
+  Progress progress(UINT64_MAX, "Parsing symbol table for %s",
+                    m_file.GetFilename().AsCString("<Unknown>"));
+
   struct symtab_command symtab_load_command = {0, 0, 0, 0, 0, 0};
   struct linkedit_data_command function_starts_load_command = {0, 0, 0, 0};
   struct dyld_info_command dyld_info = {0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0};
   // The data element of type bool indicates that this entry is thumb
   // code.
   typedef AddressDataArray<lldb::addr_t, bool, 100> FunctionStarts;
 
   // Record the address of every function/data that we add to the symtab.

lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp

 //===-- ManualDWARFIndex.cpp ----------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "Plugins/SymbolFile/DWARF/ManualDWARFIndex.h"
 #include "Plugins/Language/ObjC/ObjCLanguage.h"
 #include "Plugins/SymbolFile/DWARF/DWARFDebugInfo.h"
 #include "Plugins/SymbolFile/DWARF/DWARFDeclContext.h"
 #include "Plugins/SymbolFile/DWARF/LogChannelDWARF.h"
 #include "Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.h"
 #include "lldb/Core/Module.h"
+#include "lldb/Core/Progress.h"
 #include "lldb/Symbol/ObjectFile.h"
 #include "lldb/Utility/Stream.h"
 #include "lldb/Utility/Timer.h"
 #include "llvm/Support/ThreadPool.h"
 
 using namespace lldb_private;
 using namespace lldb;
 
for (size_t U = 0; U < dwp_info->GetNumUnits(); ++U) {
       if (auto *tu = llvm::dyn_cast<DWARFTypeUnit>(dwp_info->GetUnitAtIndex(U)))
         units_to_index.push_back(tu);
     }
   }
 
   if (units_to_index.empty())
     return;
 
+  StreamString module_desc;
+  m_module.GetDescription(module_desc.AsRawOstream(),
+                          lldb::eDescriptionLevelBrief);
+
+  // Include 2 passes per unit to index for extracting DIEs from the unit and
+  // indexing the unit, and then 8 extra entries for finalizing each index set.
+  const uint64_t total_progress = units_to_index.size() * 2 + 8;
+  Progress progress(total_progress, "Manually indexing DWARF for %s",
+                    module_desc.GetData());
+
   std::vector<IndexSet> sets(units_to_index.size());
 
   // Keep memory down by clearing DIEs for any units if indexing
   // caused us to load the unit's DIEs.
   std::vector<llvm::Optional<DWARFUnit::ScopedExtractDIEs>> clear_cu_dies(
       units_to_index.size());
   auto parser_fn = [&](size_t cu_idx) {
     IndexUnit(*units_to_index[cu_idx], dwp_dwarf, sets[cu_idx]);
+    progress.Increment();
   };
 
-  auto extract_fn = [&units_to_index, &clear_cu_dies](size_t cu_idx) {
+  auto extract_fn = [&](size_t cu_idx) {
     clear_cu_dies[cu_idx] = units_to_index[cu_idx]->ExtractDIEsScoped();
+    progress.Increment();
   };
 
   // Share one thread pool across operations to avoid the overhead of
   // recreating the threads.
   llvm::ThreadPool pool(llvm::optimal_concurrency(units_to_index.size()));
 
   // Create a task runner that extracts dies for each DWARF unit in a
   // separate thread.
   // First figure out which units didn't have their DIEs already
   // parsed and remember this.  If no DIEs were parsed prior to this index
   // function call, we are going to want to clear the CU dies after we are
   // done indexing to make sure we don't pull in all DWARF dies, but we need
   // to wait until all units have been indexed in case a DIE in one
   // unit refers to another and the indexes accesses those DIEs.
   for (size_t i = 0; i < units_to_index.size(); ++i)
     pool.async(extract_fn, i);
   pool.wait();
 
   // Now create a task runner that can index each DWARF unit in a
   // separate thread so we can index quickly.
   for (size_t i = 0; i < units_to_index.size(); ++i)
     pool.async(parser_fn, i);
   pool.wait();
 
-  auto finalize_fn = [this, &sets](NameToDIE(IndexSet::*index)) {
+  auto finalize_fn = [this, &sets, &progress](NameToDIE(IndexSet::*index)) {
     NameToDIE &result = m_set.*index;
     for (auto &set : sets)
       result.Append(set.*index);
     result.Finalize();
+    progress.Increment();
   };
 
   pool.async(finalize_fn, &IndexSet::function_basenames);
   pool.async(finalize_fn, &IndexSet::function_fullnames);
   pool.async(finalize_fn, &IndexSet::function_methods);
   pool.async(finalize_fn, &IndexSet::function_selectors);
   pool.async(finalize_fn, &IndexSet::objc_class_selectors);
   pool.async(finalize_fn, &IndexSet::globals);

lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp

 #include "llvm/ADT/Optional.h"
 #include "llvm/Support/Casting.h"
 #include "llvm/Support/Threading.h"
 
 #include "lldb/Core/Module.h"
 #include "lldb/Core/ModuleList.h"
 #include "lldb/Core/ModuleSpec.h"
 #include "lldb/Core/PluginManager.h"
+#include "lldb/Core/Progress.h"
 #include "lldb/Core/Section.h"
 #include "lldb/Core/StreamFile.h"
 #include "lldb/Core/Value.h"
 #include "lldb/Utility/ArchSpec.h"
 #include "lldb/Utility/RegularExpression.h"
 #include "lldb/Utility/Scalar.h"
 #include "lldb/Utility/StreamString.h"
 #include "lldb/Utility/Timer.h"
if (type_system_or_err) {
     type_system_or_err->SetSymbolFile(this);
   }
   return type_system_or_err;
 }
 
 void SymbolFileDWARF::InitializeObject() {
   Log *log = LogChannelDWARF::GetLogIfAll(DWARF_LOG_DEBUG_INFO);
 
   if (!GetGlobalPluginProperties()->IgnoreFileIndexes()) {
+    StreamString module_desc;
+    GetObjectFile()->GetModule()->GetDescription(module_desc.AsRawOstream(),
+                                                 lldb::eDescriptionLevelBrief);
     DWARFDataExtractor apple_names, apple_namespaces, apple_types, apple_objc;
     LoadSectionData(eSectionTypeDWARFAppleNames, apple_names);
     LoadSectionData(eSectionTypeDWARFAppleNamespaces, apple_namespaces);
     LoadSectionData(eSectionTypeDWARFAppleTypes, apple_types);
     LoadSectionData(eSectionTypeDWARFAppleObjC, apple_objc);
 
+    if (apple_names.GetByteSize() > 0 || apple_namespaces.GetByteSize() > 0 ||
+        apple_types.GetByteSize() > 0 || apple_objc.GetByteSize() > 0) {
+      Progress progress(UINT64_MAX, "Loading Apple DWARF index for %s",
+                        module_desc.GetData());
       m_index = AppleDWARFIndex::Create(
           *GetObjectFile()->GetModule(), apple_names, apple_namespaces,
           apple_types, apple_objc, m_context.getOrLoadStrData());
 
       if (m_index)
         return;
+    }
 
     DWARFDataExtractor debug_names;
     LoadSectionData(eSectionTypeDWARFDebugNames, debug_names);
     if (debug_names.GetByteSize() > 0) {
+      Progress progress(UINT64_MAX, "Loading DWARF5 index for %s",
+                        module_desc.GetData());
       llvm::Expected<std::unique_ptr<DebugNamesDWARFIndex>> index_or =
           DebugNamesDWARFIndex::Create(*GetObjectFile()->GetModule(),
                                        debug_names,
                                        m_context.getOrLoadStrData(), *this);
       if (index_or) {
         m_index = std::move(*index_or);
         return;
       }

lldb/tools/lldb-vscode/VSCode.h

 
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/DenseSet.h"
 #include "llvm/ADT/StringMap.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/JSON.h"
 #include "llvm/Support/raw_ostream.h"
 
 #include "lldb/API/SBAttachInfo.h"
 #include "lldb/API/SBBreakpoint.h"
 #include "lldb/API/SBBreakpointLocation.h"
 #include "lldb/API/SBCommandInterpreter.h"
 #include "lldb/API/SBCommandReturnObject.h"
 #include "lldb/API/SBCommunication.h"
 #include "lldb/API/SBDebugger.h"
 #include "lldb/API/SBEvent.h"
 #include "lldb/API/SBHostOS.h"
struct VSCode {
   // Serialize the JSON value into a string and send the JSON packet to
   // the "out" stream.
   void SendJSON(const llvm::json::Value &json);
 
   std::string ReadJSON();
 
   void SendOutput(OutputType o, const llvm::StringRef output);
 
+  void SendProgressEvent(uint64_t progress_id, const char *message,
+                         uint64_t completed, uint64_t total);
+
   void __attribute__((format(printf, 3, 4)))
   SendFormattedOutput(OutputType o, const char *format, ...);
 
   static int64_t GetNextSourceReference();
 
   ExceptionBreakpoint *GetExceptionBPFromStopReason(lldb::SBThread &thread);
 
   lldb::SBThread GetLLDBThread(const llvm::json::Object &arguments);

lldb/tools/lldb-vscode/VSCode.cpp

case OutputType::Telemetry:
     break;
   }
   body.try_emplace("category", category);
   EmplaceSafeString(body, "output", output.str());
   event.try_emplace("body", std::move(body));
   SendJSON(llvm::json::Value(std::move(event)));
 }
 
+// interface ProgressStartEvent extends Event {
+//   event: 'progressStart';
+//
+//   body: {
+//     /**
+//      * An ID that must be used in subsequent 'progressUpdate' and
+//      'progressEnd'
+//      * events to make them refer to the same progress reporting.
+//      * IDs must be unique within a debug session.
+//      */
+//     progressId: string;
+//
+//     /**
+//      * Mandatory (short) title of the progress reporting. Shown in the UI to
+//      * describe the long running operation.
+//      */
+//     title: string;
+//
+//     /**
+//      * The request ID that this progress report is related to. If specified a
+//      * debug adapter is expected to emit
+//      * progress events for the long running request until the request has
+//      been
+//      * either completed or cancelled.
+//      * If the request ID is omitted, the progress report is assumed to be
+//      * related to some general activity of the debug adapter.
+//      */
+//     requestId?: number;
+//
+//     /**
+//      * If true, the request that reports progress may be canceled with a
+//      * 'cancel' request.
+//      * So this property basically controls whether the client should use UX
+//      that
+//      * supports cancellation.
+//      * Clients that don't support cancellation are allowed to ignore the
+//      * setting.
+//      */
+//     cancellable?: boolean;
+//
+//     /**
+//      * Optional, more detailed progress message.
+//      */
+//     message?: string;
+//
+//     /**
+//      * Optional progress percentage to display (value range: 0 to 100). If
+//      * omitted no percentage will be shown.
+//      */
+//     percentage?: number;
+//   };
+// }
+//
+// interface ProgressUpdateEvent extends Event {
+//   event: 'progressUpdate';
+//
+//   body: {
+//     /**
+//      * The ID that was introduced in the initial 'progressStart' event.
+//      */
+//     progressId: string;
+//
+//     /**
+//      * Optional, more detailed progress message. If omitted, the previous
+//      * message (if any) is used.
+//      */
+//     message?: string;
+//
+//     /**
+//      * Optional progress percentage to display (value range: 0 to 100). If
+//      * omitted no percentage will be shown.
+//      */
+//     percentage?: number;
+//   };
+// }
+//
+// interface ProgressEndEvent extends Event {
+//   event: 'progressEnd';
+//
+//   body: {
+//     /**
+//      * The ID that was introduced in the initial 'ProgressStartEvent'.
+//      */
+//     progressId: string;
+//
+//     /**
+//      * Optional, more detailed progress message. If omitted, the previous
+//      * message (if any) is used.
+//      */
+//     message?: string;
+//   };
+// }
+
+void VSCode::SendProgressEvent(uint64_t progress_id, const char *message,
+                               uint64_t completed, uint64_t total) {
+  enum ProgressEventType {
+    progressInvalid,
+    progressStart,
+    progressUpdate,
+    progressEnd
+  };
+  const char *event_name = nullptr;
+  ProgressEventType event_type = progressInvalid;
+  if (completed == 0) {
+    event_type = progressStart;
+    event_name = "progressStart";
+  } else if (completed == total) {
+    event_type = progressEnd;
+    event_name = "progressEnd";
+  } else if (completed < total) {
+    event_type = progressUpdate;
+    event_name = "progressUpdate";
+  }
+  if (event_type == progressInvalid)
+    return;
+
+  llvm::json::Object event(CreateEventObject(event_name));
+  llvm::json::Object body;
+  std::string progress_id_str;
+  llvm::raw_string_ostream progress_id_strm(progress_id_str);
+  progress_id_strm << progress_id;
+  progress_id_strm.flush();
+  body.try_emplace("progressId", progress_id_str);
+  if (event_type == progressStart) {
+    EmplaceSafeString(body, "title", message);
+    body.try_emplace("cancellable", false);
+  }
+
+  if (0 < total && total < UINT64_MAX) {
+    uint32_t percentage = (uint32_t)(((float)completed / (float)total) * 100.0);
+    body.try_emplace("percentage", percentage);
+  }
+  event.try_emplace("body", std::move(body));
+  SendJSON(llvm::json::Value(std::move(event)));
+}
+
 void __attribute__((format(printf, 3, 4)))
 VSCode::SendFormattedOutput(OutputType o, const char *format, ...) {
   char buffer[1024];
   va_list args;
   va_start(args, format);
   int actual_length = vsnprintf(buffer, sizeof(buffer), format, args);
   va_end(args);
   SendOutput(

lldb/tools/lldb-vscode/lldb-vscode.cpp

void request_initialize(const llvm::json::Object &request) {
   // 'disconnect' request.
   body.try_emplace("supportTerminateDebuggee", true);
   // The debug adapter supports the delayed loading of parts of the stack,
   // which requires that both the 'startFrame' and 'levels' arguments and the
   // 'totalFrames' result of the 'StackTrace' request are supported.
   body.try_emplace("supportsDelayedStackTraceLoading", true);
   // The debug adapter supports the 'loadedSources' request.
   body.try_emplace("supportsLoadedSourcesRequest", false);
+  // The debug adapter supports sending progress reporting events.
+  body.try_emplace("supportsProgressReporting", true);
 
   response.try_emplace("body", std::move(body));
   g_vsc.SendJSON(llvm::json::Value(std::move(response)));
 }
 
 llvm::Error request_runInTerminal(const llvm::json::Object &launch_request) {
   g_vsc.is_attach = true;
   lldb::SBAttachInfo attach_info;
#else
 
   std::string error = std::strerror(errno);
   comm_channel.NotifyError(error);
   llvm::errs() << error << "\n";
   exit(EXIT_FAILURE);
 #endif
 }
 
+void LLDBProgressCallback(uint64_t progress_id, const char *message,
+                          uint64_t completed, uint64_t total, void *baton) {
+  g_vsc.SendProgressEvent(progress_id, message, completed, total);
+}
+
 int main(int argc, char *argv[]) {
   llvm::SmallString<256> program_path(argv[0]);
   llvm::sys::fs::make_absolute(program_path);
   g_vsc.debug_adaptor_path = program_path.str().str();
 
   LLDBVSCodeOptTable T;
   unsigned MAI, MAC;
   llvm::ArrayRef<const char *> ArgsArr = llvm::makeArrayRef(argv + 1, argc);
if (llvm::opt::Arg *comm_file = input_args.getLastArg(OPT_comm_file)) {
       llvm::errs() << "\"--launch-target\" requires \"--comm-file\" to be "
                       "specified\n";
       exit(EXIT_FAILURE);
     }
   }
 
   // Initialize LLDB first before we do anything.
   lldb::SBDebugger::Initialize();
+  lldb::SBDebugger::SetProgressCallback(LLDBProgressCallback, nullptr);
 
   RegisterRequestCallbacks();
 
   int portno = -1;
 
   if (input_args.hasArg(OPT_help)) {
     printHelp(T, llvm::sys::path::filename(argv[0]));
     return 0;