[cherry-pick][lldb] Implement missing queue overloads from ThreadMemory #10342
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
ThreadMemory attempts to be a class abstracting the notion of a "fake" Thread that is backed by a "real" thread. According to its documentation, it is meant to be a class forwarding most methods to the backing thread, but it does so only for a handful of methods. Along the way, it also tries to represent a Thread that may or may not have a different name, and may or may not have a different queue from the backing thread. This can be problematic for a couple of reasons: 1. It forces all users into this optional behavior. 2. The forwarding behavior is incomplete: not all methods are currently being forwarded properly. Some of them involve queues and seem to have been intentionally left unimplemented. This commit creates the following separation: ThreadMemory <- ThreadMemoryProvidingName <- ThreadMemoryProvidingNameAndQueue ThreadMemory captures the notion of a backed thread that _really_ forwards all methods to the backing thread. (Missing methods should be implemented in a later commit, and allowing them to be implemented without changing behavior of other derived classes is the main purpose of this refactor). ThreadMemoryProvidingNameAndQueue is a ThreadMemory that allows users to override the thread name. If a name is present, it is used; otherwise the forwarding behavior is used. ThreadMemoryProvidingNameAndQueue is a ThreadMemoryProvidingName that allows users to override queue information. If queue information is present, it is used; otherwise, the forwarding behavior is used. With this separation, we can more explicitly implement missing methods of the base class and override them, if needed, in ThreadMemoryProvidingNameAndQueue. But this commit really is NFC, no new methods are implemented and no method implementation is changed. (cherry picked from commit 65ad02b)
This commit makes ThreadMemory a real "forwarder" class by implementing the missing queue methods: they will just call the corresponding backing thread method. To make this patch NFC(*) and not change the behavior of the Python OS plugin, NamedThreadMemoryWithQueue also overrides these methods to simply call the `Thread` method, just as it was doing before. This also makes it obvious that there are missing pieces of this class if it were to provide full queue support. (*) This patch is NFC in the sense that all llvm.org plugins will not have any behavior change, but downstream consumers of ThreadMemory will benefit from the newly implemented forwarding methods. (cherry picked from commit 07c82b1)
felipepiovezan
force-pushed
the
felipe/cherrypick_missing_mem
branch
from
bc072ea to
16733ed
Compare
|
@swift-ci test |
adrian-prantl
approved these changes
Mar 25, 2025
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.
This is a cherry-pick of
llvm#132905
llvm#132906
With an extra commit updating the swift plugin to account for the new class name (and a test)
rdar://147154570