clayborg added a comment. In D93895#2480324 <https://reviews.llvm.org/D93895#2480324>, @augusto2112 wrote:
>> So it might be nice to add support for vAttachOrWait, along with the query >> packet, in this patch if you have the time? > > Hi @clayborg, I saw that implementing vAttachOrWait was already 90% done, so > I did just that :) The current patch as-is supports vAttachOrWait. My > question was more on the lines of if keeping the query for vAttachOrWait made > sense now that it's implemented on Linux. Are there be other targets where it > isn't implemented? Best to not change anything if we don't know. Yes there are other GDB servers out there that may not implement vAttachOrWait, so I would vote to not change anything. > I also left a comment with a couple of questions earlier, but I think it got > hidden by my last comment. I'll repost it here: > >> @clayborg I've added support for the 'waitfor-interval' and >> 'waitfor-duration' flags. Yesterday I thought they existed in macOS, but now >> I'm not so sure, as I couldn't find them on "Options.td". They were added in >> 2009, so maybe they did exist but were dropped later. Or I just didn't look >> at the right place. >> >> A couple of questions: >> >> - About the default polling interval, I've set it to 1ms for now, but I >> found that (in my system) this keeps a core at 100%. 10ms keeps still keeps >> it at around 80%, and 100ms keeps it at around 35%. Maybe 100ms is a good >> balance? debugserver in the llvm sources uses 1ms as its default. I would vote to be as quick as possible so we can catch the process as soon as possible. The option --waitfor-interval specifies the time in microseconds, so we will want to mirror that. So even though it does peg one core at 100%, I think it might be worth it to catch the process sooner than later. >> - On "Options.td", besides the "process attach" command, there's also a >> "platform process attach". I haven't touched it since I'm not familiar with >> the platform command. Should I add these flags to the platform counterpart >> as well? Just "process attach" is fine to start with. The platform layer doesn't always have to use a GDB server, so adding those options there could end up being hard for some platforms to implement. In fact I would mind if we don't even add these options to "process attach" and just always use the defaults, because as you say, if someone selects or replaces the lldb-server with an older one, then we don't want the launching of the lldb-server to fail because it doesn't recognize an option that is being passed to it. The better way to fix this would be to make a new attach wait packet that takes these as arguments to the packet, but we don't need to do that. > Could you confirm for me that 'waitfor-interval' and 'waitfor-duration' don't > exist on macOS? If I'm sending a different message format than what's done on > macOS that'd be an issue. Here lies the problem that I mentioned above. I would like to avoid having to launch lldb-server with any arguments so that we continue to work with older lldb-servers. So maybe we just rely on defaults for now and avoid having to add any new arguments to "process attach" and to the launching of the lldb-server? Let me know your thoughts. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93895/new/ https://reviews.llvm.org/D93895 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
