[Lldb-commits] [PATCH] D132578: [lldb] [Core] Use thread for Communication::Write() as well

mgorny created this revision. mgorny added reviewers: labath, krytarowski, emaste, jingham. Herald added a subscriber: arichardson. Herald added a project: All. mgorny requested review of this revision. When read thread is enabled, perform all the writes in the thread as well. Most importantly, t

[Lldb-commits] [PATCH] D132578: [lldb] [Core] Use thread for Communication::Write() as well

mgorny updated this revision to Diff 455295. mgorny added a comment. Update to the version with error support. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D132578/new/ https://reviews.llvm.org/D132578 Files: lldb/include/lldb/Core/Communication.h lldb/source/Core/Communication.cpp

[Lldb-commits] [PATCH] D132578: [lldb] [Core] Use thread for Communication::Write() as well

labath added a comment. Can you explain where you're heading with this? I can imagine that something like this might be necessary in some cases, but I also don't see anything inherently wrong with concurrent reads and writes to the same connection. CHANGES SINCE LAST ACTION https://reviews.l

[Lldb-commits] [PATCH] D132578: [lldb] [Core] Use thread for Communication::Write() as well

mgorny added a comment. Well, back when working on the async thread, you've indicated that it's a bad idea to read from one thread while writing from another, so I've basically focused on getting to the point when all reads and writes are guaranteed to happen from a single thread. CHANGES SIN

[Lldb-commits] [PATCH] D132578: [lldb] [Core] Use thread for Communication::Write() as well

mgorny added a comment. (with the final goal of using `StartReadThread()` inside gdb-remote comms to take care of reading the data for concurrent process plugin instances) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D132578/new/ https://reviews.llvm.org/D132578 __

[Lldb-commits] [PATCH] D132578: [lldb] [Core] Use thread for Communication::Write() as well

mgorny updated this revision to Diff 455952. mgorny added a comment. Rebase. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D132578/new/ https://reviews.llvm.org/D132578 Files: lldb/include/lldb/Core/Communication.h lldb/source/Core/Communication.cpp lldb/unittests/Core/Communicati

[Lldb-commits] [PATCH] D132578: [lldb] [Core] Use thread for Communication::Write() as well

labath added a comment. In D132578#3751968 , @mgorny wrote: > Well, back when working on the async thread, you've indicated that it's a bad > idea to read from one thread while writing from another, so I've basically > focused on getting to the point wh

[Lldb-commits] [PATCH] D132578: [lldb] [Core] Use thread for Communication::Write() as well

mgorny added a comment. Well, my "baseline" idea was something like that: having a shared (e.g. via `shared_ptr`) `m_gdb_comm`, and running async thread as part of that. The thread would read packets from remote and split them into two groups: asynchronous events (i.e. stop reasons and related

[Lldb-commits] [PATCH] D132578: [lldb] [Core] Use thread for Communication::Write() as well

mgorny added a comment. Ok, so I guess I'm back the original plan of using async thread to do the packet exchanges. Right now, the rough plan is to: 1. Try to clean up / streamline the comms API a bit. 2. Introduce a wrapper between the client and the actual comms to clearly split the public pa

[Lldb-commits] [PATCH] D132578: [lldb] [Core] Use thread for Communication::Write() as well

labath added a comment. In D132578#3769078 , @mgorny wrote: > Ok, so I guess I'm back the original plan of using async thread to do the > packet exchanges. Right now, the rough plan is to: > > 1. Try to clean up / streamline the comms API a bit. > 2. Int

[Lldb-commits] [PATCH] D132578: [lldb] [Core] Use thread for Communication::Write() as well

mgorny added a comment. In D132578#3770124 , @labath wrote: > Seems reasonable. Just be aware that the packet RTT is very important for > some, so we will either make sure that the extra thread is not used in the > base case (no processes running), or s

[Lldb-commits] [PATCH] D132578: [lldb] [Core] Use thread for Communication::Write() as well

mgorny updated this revision to Diff 458017. mgorny added a comment. Rebase on top of D131160 . CHANGES SINCE LAST ACTION https://reviews.llvm.org/D132578/new/ https://reviews.llvm.org/D132578 Files: lldb/include/lldb/Core/Communication.h lldb/source/Cor