forked from llvm/llvm-project
-
Notifications
You must be signed in to change notification settings - Fork 341
[🍒 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
JDevlieghere
merged 6 commits into
swift/release/6.1
from
cherrypick-swift/release/6.1-a939a9fd53d9
Jan 27, 2025
Merged
[🍒 swift/release/6.1] [LLDB-DAP] Send Progress update message over DAP (#123837) #9874
JDevlieghere
merged 6 commits into
swift/release/6.1
from
cherrypick-swift/release/6.1-a939a9fd53d9
Jan 27, 2025
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
@swift-ci test |
adrian-prantl
approved these changes
Jan 23, 2025
db7ec12
to
9e38a00
Compare
@swift-ci test |
9e38a00
to
b1ab2e3
Compare
@swift-ci test |
1 similar comment
@swift-ci test |
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)  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  Now  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)
1c85dd8
to
46aa7a2
Compare
@swift-ci test |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Now
Tested with my progress tester
command, testing 10 events 5 seconds apart 1-10
(cherry picked from commit a939a9f)