[Lldb-commits] [PATCH] D11415: Add jstopinfo support to llgs

[Jason Molenda]

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 would have been weird to adopt it in this one place.

J

> On Jul 30, 2015, at 5:09 AM, Pavel Labath <labath at 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 <labath at 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 at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits