This commit integrates MainLoop into NativeProcessLinux. By registering a SIGCHLD handler with
the llgs main loop, we can get rid of the special monitor thread in NPL, which saves as a lot of
thread ping-pong when responding to client requests (e.g. qThreadInfo processing time has been
reduced by about 40%). It also makes the code simpler, IMHO.
Details
- Reviewers
chaoren ovyalov clayborg tberghammer - Commits
- rG19cbe96a4575: [NativeProcessLinux] Integrate MainLoop
rG827965c33c5a: [NativeProcessLinux] Integrate MainLoop
rLLDB242783: [NativeProcessLinux] Integrate MainLoop
rLLDB242305: [NativeProcessLinux] Integrate MainLoop
rL242305: [NativeProcessLinux] Integrate MainLoop
Diff Detail
- Repository
- rL LLVM
Event Timeline
include/lldb/Host/common/NativeProcessProtocol.h | ||
---|---|---|
19 ↗ | (On Diff #29583) | Can you use forward declaration here? |
source/Plugins/Process/Linux/NativeProcessLinux.cpp | ||
3302 ↗ | (On Diff #29583) | indentation. |
3303 ↗ | (On Diff #29583) | Please consider storing errno as a local variable before logging - if log->Printf will be failing by any reason we don't want it to overwrite errno. |
3310 ↗ | (On Diff #29583) | s/NULL/nullptr |
source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp | ||
102 ↗ | (On Diff #29583) | Since you're removing Terminate method from it's seems feasible to remove this destructor impl as well. |
- Address review comments
include/lldb/Host/common/NativeProcessProtocol.h | ||
---|---|---|
19 ↗ | (On Diff #29583) | The simple forward declaration class MainLoop; will not work because MainLoop is a typedef and you will get redefinition errors. We could try to do something clever here, but I am not sure it is worth it. I could also have the interface take MainLoopBase and forward declare that, but then I would have to upcast the object to MainLoopPosix in NPL, which i also don't like much. |
source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp | ||
102 ↗ | (On Diff #29583) | Note that I haven't removed the base method NativeProcessProtocol::Terminate(). However, I think this is a good idea, so I will interpret this comment as a request to remove that as well. :) |
LGTM
source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp | ||
---|---|---|
98–99 ↗ | (On Diff #29660) | Thanks) |