labath wrote:
I also haven't looked at it in detail yet, but I think it makes sense overall.
I'm not saying it has to be done -- it depends on where we want to take this.
Doing everything on one thread makes it easy to avoid races and it's a strategy
I generally like, but it also kind of goes against some of our other goals of
making things faster by doing work in parallel. But then again, it's only "kind
of" since it's still possible to introduce more controlled parallelism to -- I
don't know -- fetch inferior threads in parallel if it makes sense. And that
may actually be better in the long run.
Since I'm not going to be doing this work, I'll let you figure out the
direction here.
I think the patch could use some splitting up. There's a bunch of typo fixes
and other things that could go separately. And what's up with all the test
changes -- shouldn't this be "NFC" ?
Jonas is right that class isn't (completely) thread safe. That's because it
originally started out as a completely single-process thing -- which means
there are no threads to synchronise. It's grown beyond that now though, and now
it has some synchronization. Specifically, `AddPendingCallback` is thread-safe
(but not async-signal-safe -- I think that's the part that Jonas is
remembering). The thing that's not thread-safe is `RegisterReadObject`. Right
now it's possible to work around that by doing something like
`loop.AddPendingCallback([&] { loop.RegisterReadObject(); });` but if you find
yourself needing to do that, maybe we can add that directly to the MainLoop
class.
https://github.com/llvm/llvm-project/pull/139669
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
