jimingham wrote:

The goal here is excellent!  Not only is this a performance problem but if you 
call the version of GetValueFromExpression that doesn't take an expression 
options - as the code you are replacing does - you get the default value for 
"TryAllThreads" - which is `true`.  So if you are unlucky and the expression 
takes a little bit of time (maybe your machine is heavily loaded or something) 
then printing this summary could cause other threads to make progress out from 
under you.  In a GUI, that can result in the not helpful behavior where you 
stop at some point in Thread A, switch to Thread B - at which point the GUI 
loads the variables and runs the formatter for you and in doing so Thread A 
gets a chance to run.  Then you switch back to Thread A and it wasn't where you 
left it.  Bad debugger!

We should not use expressions in data formatters because they make them slow as 
you have seen.  But we for sure should never call an expression with 
`TryAllThreads` set to true in a data formatter.  If you are going to use 
expressions at all you should do it on only one thread and make sure that along 
the path of the function you call nobody takes locks that might deadlock the 
expression against other threads...  But we should try hard not to make that 
necessary, and shouldn't do so in ones we ship.

Another general rule is that if you are adding a significant algorithm to an SB 
API implementation, you should really make an API in the underlying 
lldb_private API and then call that.  We used to have all sorts of SB API 
specific implementations, which leads to code duplication and subtle 
differences in behavior depending on whether you find your way to the SB or 
private implementation.  We try not to do that anymore.

Can you do this instead with a ValueObject::CreateBooleanValue, and then just 
wrap that in the SBValue::CreateBooleanValue call?

https://github.com/llvm/llvm-project/pull/108414
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to