[Lldb-commits] [PATCH] D103271: [lldb/Target] Select most relevant frame only in case of signal

[Levon Ter-Grigoryan via Phabricator via lldb-commits]

PatriosTheGreat requested review of this revision.
PatriosTheGreat added a comment.

Thanks for feedback.

I made a small sample: https://pastebin.com/F8nde1zi
Compile command: clang++ -O0 -g -pthread sample.cpp 
LLDB command: breakpoint set -f sample.cpp -l 24 --condition 'i == 100' 
This break should not be hit due to the condition.

I got followed results for local and remote run:
LLDB 11:
Local:
Time difference = 10 330 [ms]
Remote:
Time difference = 189 707 [ms]

Patched (turned off select of the most relevant frame for threads which were stopped without a reason):
Local:
Time difference = 7 865 [ms]
Remote:
Time difference = 13 630 [ms]

Looks like there is a performance improvement even with local run, but with remote run it becomes much more significant.

In D103271#2785450 <https://reviews.llvm.org/D103271#2785450>, @jingham wrote:

> It is incorrect to say that SelectMostRelevantFrame is only relevant for asserts.  The FrameRecognizer is a generic mechanism, and you have no a priori way to know what kinds of thread stops it wants to operate on.  So this patch is not correct.
>
> Other than calling the Frame Recognizers, SelectMostRelevantFrame does NOT trigger a stack walk itself.
>
> So if there is a frame recognizer that is doing a frame walk on threads that aren't relevant to it, then changing the frame recognizer to check that the stop reason of its thread makes sense before doing any work seems a better solution to your problem.

>From performance analyzer I see that SelectMostRelevantFrame calls StackFrameList::GetFrameAtIndex -> StackFrameList::GetFramesUpTo -> UnwindLLDB::DoGetFrameInfoAtIndex -> UnwindLLDB::AddFirstFrame
UnwindLLDB::AddFirstFrame calls two expensive methods:  UnwindLLDB::UpdateUnwindPlanForFirstFrameIfInvalid (takes around 2/3 of execution time) and UnwindLLDB::RegisterContextUnwind (takes around 1/3 of execution time)

Does frame recognition relevant for threads which were stopped without any reason?
May I filter this logic like this:

  lldb::StopReason stop_reason = GetStopReason();
  if (stop_reason != lldb::StopReason::eStopReasonNone)
    SelectMostRelevantFrame();

If this is also an incorrect solution may we somehow select most relevant frame without calling of  UnwindLLDB::DoGetFrameInfoAtIndex?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D103271/new/

https://reviews.llvm.org/D103271