Thanks for the explanation. I will be away for two weeks, but then I will look into this again, if somebody does not beat me to it :)
cheers, pl On 31 July 2015 at 17:28, Jason Molenda <ja...@molenda.com> wrote: > Yes, we talked about this approach a few times. Greg went with the jstopinfo > addition to the T packet to keep the changes relatively small, but we're all > in agreement that we should create a new JSON formatted stop info packet and > dump the T packet altogether. > > That's one of those changes where we're taking a big step away from > traditional gdb-remote protocol which is the only reason Greg hesitated to do > it. There's always the concern about lldb compatibility with traditional > gdb-remote implementations -- we don't test that very > thoroughly/frequently/consistently, and it can be easy to add assumptions > that we have the lldb extensions to the protocol. > > None of this is an argument against a JSON formatted thread reply packet. > Greg and I both think that this is the best approach - it was more work than > he wanted to take on when he was implementing this addition to the T packet. > > J > >> On Jul 31, 2015, at 2:46 AM, Pavel Labath <lab...@google.com> wrote: >> >> Thank you all for the replies, very interesting information there. >> Given that the register caching approach will not work everywhere, I >> agree we should stick to sending the PC on all platforms for >> consistency. >> >> Regarding the size of the stop-reply packet, I have a question. Is >> there any reason we are tied to the current format of the stop-reply >> packet as a whole? Couldn't we just change it as a whole? For example, >> we could add a QThreadStopInJSON packet, which will switch the server >> to send the stop-replies in the current jThreadsInfo format (perhaps >> prefixed by a T or something, to make it more easily detectable). This >> would solve the problem of the size increase, and it would enable us >> to easily add more data to the packet in the future. >> >> What do you think? >> >> cheers, >> pl >> >> >> On 30 July 2015 at 19:10, Jason Molenda <ja...@molenda.com> wrote: >>> We were discussing the same thing last week - I think we all agree that >>> approach #1 is the way to go. My main concern here is that jstopinfo is >>> ascii-hex encoded. JSON is already pretty verbose (particularly with all >>> the numbers being base 10, sigh) but then we double the size of the string. >>> Originally when we listed the stop reasons for all the threads were were >>> including a list of all threads and their stop reasons (even if it is >>> 'none') and the payload was enormous. I was playing with ideas like given >>> that the T packet already gives us a list of thread ids outside the >>> jstopinfo payload, maybe we could include an array of thread pc values but >>> that was not a good idea even though it would be compact. I think we'll >>> need to include a dictionary of threadid:pc's in there. We could have >>> mitigated the expansion a little by adopting base64 encoding instead of >>> asciihex (133% increase as opposed to 200% increase) but base64 isn't used >>> anywhere else in the protocol so it w! ould have been weird to adopt it in this one place. >>> >>> J >>> >>>> On Jul 30, 2015, at 5:09 AM, Pavel Labath <lab...@google.com> wrote: >>>> >>>> Hello Greg, Jim, >>>> >>>> after adding jstopinfo support to LLGS, i am still getting a bunch of >>>> register read packets which access the program counter. These are >>>> coming from Thread::SetupForResume(), where we need to check whether a >>>> thread is on a breakpoint before letting it run. I would like to get >>>> rid of those packets (my preliminary tests show I can gain about 10% >>>> stepping speed improvement on my test app with 20 threads). I see two >>>> ways to go about this: >>>> 1. send the program counters as a part of the jstopinfo field. I will >>>> need to change the format of the field a bit, because the current >>>> assumption is that you do not include the threads which don't have a >>>> stop reason there, but we need the registers for every thread. >>>> 2. cache the registers on the client side. These queries happen after >>>> we have previously done a vCont:s (as a part of ThreadPlanStepOver), >>>> so the client can determine that the other threads have not executed, >>>> and the registers are unchanged. We would still avoid caching the stop >>>> reason, since on OSX this can change even if the thread is not >>>> running, but I hope the registers remain the same. >>>> >>>> All in all, the first approach is probably more easier to implement, >>>> but the second one sounds better to me architecturally, and has the >>>> benefit of caching all registers, and not just the PC. >>>> >>>> Do you have any thoughts on this? >>>> >>>> cheers, >>>> pl >>>> >>>> >>>> On 23 July 2015 at 10:10, Pavel Labath <lab...@google.com> wrote: >>>>> This revision was automatically updated to reflect the committed changes. >>>>> labath marked 2 inline comments as done. >>>>> Closed by commit rL242997: Add jstopinfo support to llgs (authored by >>>>> labath). >>>>> >>>>> Changed prior to commit: >>>>> http://reviews.llvm.org/D11415?vs=30348&id=30464#toc >>>>> >>>>> Repository: >>>>> rL LLVM >>>>> >>>>> http://reviews.llvm.org/D11415 >>>>> >>>>> Files: >>>>> lldb/trunk/source/Plugins/Process/Linux/NativeProcessLinux.cpp >>>>> lldb/trunk/source/Plugins/Process/Linux/NativeThreadLinux.cpp >>>>> lldb/trunk/source/Plugins/Process/Linux/NativeThreadLinux.h >>>>> lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp >>>>> >>>> _______________________________________________ >>>> lldb-commits mailing list >>>> lldb-commits@cs.uiuc.edu >>>> http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits >>> > _______________________________________________ lldb-commits mailing list lldb-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits
