Skip to content

[🍒 swift/release/6.1] [LLDB-DAP] Send Progress update message over DAP (#123837) #9874

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

Merged

Conversation

JDevlieghere
Copy link

When testing my SBProgress DAP PR (llvm#123826), I noticed Progress update messages aren't sent over DAP. This patch adds the lldb progress event's message to the body when sent over DAP.

Before

image

Now

image

Tested with my progress tester
command
, testing 10 events 5 seconds apart 1-10

(cherry picked from commit a939a9f)

@JDevlieghere JDevlieghere requested a review from a team as a code owner January 23, 2025 20:52
@JDevlieghere
Copy link
Author

@swift-ci test

@JDevlieghere JDevlieghere force-pushed the cherrypick-swift/release/6.1-a939a9fd53d9 branch from db7ec12 to 9e38a00 Compare January 24, 2025 17:44
@JDevlieghere
Copy link
Author

@swift-ci test

@JDevlieghere JDevlieghere force-pushed the cherrypick-swift/release/6.1-a939a9fd53d9 branch from 9e38a00 to b1ab2e3 Compare January 24, 2025 19:31
@JDevlieghere
Copy link
Author

@swift-ci test

1 similar comment
@JDevlieghere
Copy link
Author

@swift-ci test

labath and others added 6 commits January 27, 2025 09:23
For high-frequency multithreaded progress reports, the contention of
taking the progress mutex and the overhead of generating event can
significantly slow down the operation whose progress we are reporting.

This patch adds an (optional) capability to rate-limit them. It's
optional because this approach has one drawback: if the progress
reporting has a pattern where it generates a burst of activity and then
blocks (without reporting anything) for a large amount of time, it can
appear as if less progress was made that it actually was (because we
only reported the first event from the burst and dropped the other
ones).

I've also made a small refactor of the Progress class to better
distinguish between const (don't need protection), atomic (are used on
the hot path) and other (need mutex protection) members.

(cherry picked from commit 0dbdc23)
…m#119052)

Recently I've been working on a lot of internal Python tooling, and in
certain cases I want to report async to the script over DAP. Progress.h
already handles this, so I've exposed Progress via the SB API so Python
scripts can also update progress objects.

I actually have no idea how to test this, so I just wrote a [toy command
to test
it](https://gist.github.com/Jlalond/48d85e75a91f7a137e3142e6a13d0947)

![image](https://github.com/user-attachments/assets/7317cbb8-9145-4fdb-bacf-9864bf50c467)

I also copied the first section of the extensive Progress.h class
documentation to the docstrings.

(cherry picked from commit 6b048ae)
As feedback on llvm#119052, it was recommended I add a new bit to delineate
internal and external progress events. This patch adds this new
category, and sets up Progress.h to support external events via
SBProgress.

(cherry picked from commit 774c226)
When testing my SBProgress DAP PR (llvm#123826), I noticed Progress update
messages aren't sent over DAP. This patch adds the lldb progress event's
message to the body when sent over DAP.

Before

![image](https://github.com/user-attachments/assets/404adaa8-b784-4f23-895f-cd3625fdafad)

Now

![image](https://github.com/user-attachments/assets/eb1c3235-0936-4e36-96e5-0a0ee60dabb8)

Tested with my [progress tester
command](https://gist.github.com/Jlalond/48d85e75a91f7a137e3142e6a13d0947),
testing 10 events 5 seconds apart 1-10

(cherry picked from commit a939a9f)
…lvm#123826)

Recently I added SBProgress (llvm#119052), and during that original commit I
tested if the progress event was sent over LLDB-DAP, and it was. However
upon the suggestion of @JDevlieghere and @labath we added an external
category (llvm#120171), which I did not test.

This small patch wires up DAP to listen for external events by default,
and adds the external category to the SBDebugger enumeration.

(cherry picked from commit b9813ce)
@JDevlieghere JDevlieghere force-pushed the cherrypick-swift/release/6.1-a939a9fd53d9 branch from 1c85dd8 to 46aa7a2 Compare January 27, 2025 17:25
@JDevlieghere
Copy link
Author

@swift-ci test

@JDevlieghere JDevlieghere merged commit ff59629 into swift/release/6.1 Jan 27, 2025
3 checks passed
@JDevlieghere JDevlieghere deleted the cherrypick-swift/release/6.1-a939a9fd53d9 branch January 27, 2025 21:34
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.

4 participants