labath added inline comments.
================
Comment at: lldb/source/Target/TargetProperties.td:183
Desc<"A path to a python OS plug-in module file that contains a
OperatingSystemPlugIn class.">;
+ def PluginReportsAllThreads: Property<"plugin-reports-all-threads",
"Boolean">,
+ Global,
----------------
jingham wrote:
> labath wrote:
> > jingham wrote:
> > > labath wrote:
> > > > jingham wrote:
> > > > > labath wrote:
> > > > > > If this is relevant only for os plugins, then it would be good to
> > > > > > reflect that in the name as well.
> > > > > I thought about that. In the future a regular Process plugin might
> > > > > decide it was too expensive to report all threads as well. There's
> > > > > nothing in this patch that wouldn't "just work" with that case as
> > > > > well. Leaving the OS out was meant to indicate this was about how
> > > > > the Process plugin OR any of its helpers (e.g. the OS Plugin)
> > > > > produces threads.
> > > > Well, I am hoping that if we ever extend this support to the regular
> > > > process plugins, we will implement it in a way where the plugin itself
> > > > can tell us whether it is operating/supporting this mode (which I guess
> > > > would involve a new gdb-remote packet and some specification of what
> > > > exactly should happen when it gets sent), instead of relying on the
> > > > user to set this correctly.
> > > >
> > > > I mean, in an ideal world this is I would want to happen with the
> > > > python plugins as well, but it seems that we are stuck with some
> > > > existing plugins which already do that. However, I wouldn't want to
> > > > plan on the same thing happening again. :)
> > > Right, I put this in mostly to help backwards compatibility. For
> > > instance, another OS Plugin I know about handles some older cooperative
> > > threading scheme. That one does report all threads on each stop. I
> > > didn't want to force them to do anything to keep their plugin working as
> > > well as it did before. That's also why I set the default to true here.
> > >
> > > Even when we have plugins that actually support not reporting all
> > > threads, you could imagine somebody having a Process plugin that supports
> > > both modes - particularly early in the development of its support for
> > > this feature, and in some corner cases the "doesn't report all threads"
> > > mode has some subtle problem. Having this setting will allow people to
> > > get the slower but more assuredly correct behavior till it works 100%
> > > reliably. So I still think the setting has some value.
> > >
> > > But I agree, going forward there should be some kind of handshake between
> > > the ThreadPlanStackMap and the Process Plugin, either a "I've reported
> > > all threads now" which could trigger a prune, or a "Is TID X still alive"
> > > which the generic code could use to balance the cost of keeping outdated
> > > stacks alive against when we want to ask about all threads.
> > All of this sounds fine, and I wouldn't mind having a setting like that,
> > even after the support for partial thread lists is considered "stable".
> > However, that sounds like a different setting to me -- that's like a
> > directive to the process plugin about how it should behave, whereas this
> > setting is more like a statement of fact about what the plugin does.
> >
> > The setting could be later repurposed to do both, but it's not clear to me
> > whether that is useful. Like, since we already support plugin-specific
> > settings, the plugin which decides to implement both modes of operation
> > could expose that as a custom setting. That way, one wouldn't get the
> > impression that this setting applies to any process plugin...
> Okay. I don't want to have to design that right now.
>
> Since this is just a workaround for the fact that the extant OS plugins don't
> have a way to tell me this fact, I'll go back to using this just for OS
> plugins. In that case, I think I should move it to experimental. Once we
> add an API to the various plugins to advertise how they work, it won't be
> needed. The benefit of experimental settings is that their absence doesn't
> cause the "settings set" to raise an error, so people can leave this in their
> lldbinit's and it won't cause problems once we use it.
>
> Now I just have to figure out how experimental settings work in the new
> tablegen way of doing properties...
An experimental setting sounds good to me.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D76814/new/
https://reviews.llvm.org/D76814
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
