[Lldb-commits] [PATCH] D102092: [lldb] Enable -Wmisleading-indentation

2021-05-31 Thread Danila Malyutin via Phabricator via lldb-commits
danilaml added a comment.

I remember some noise due to this option (albeit on GCC), because it stops 
tracking indentation when the number of lines is too big, as it is for files 
including TableGen'ed inc files. Not sure if there is a way to improve it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102092

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


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

2021-05-31 Thread 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 , @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

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D103391: [lldb] Add missing reproducer instrumentation to some SB classes

2021-05-31 Thread Bruce Mitchener via Phabricator via lldb-commits
brucem added inline comments.



Comment at: lldb/source/API/SBDebugger.cpp:165
  bool _debugger_specific) {
+  LLDB_RECORD_STATIC_METHOD(
+  const char *, SBDebugger, GetProgressFromEvent,

This one is already present in the function but near the end of the function 
due to our parameters.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103391

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] 24ee6d3 - [lldb][NFC] Remove unused var in SBDebugger::GetInternalVariableValue

2021-05-31 Thread Raphael Isemann via lldb-commits

Author: Raphael Isemann
Date: 2021-05-31T11:19:02+02:00
New Revision: 24ee6d3d3c6215db11e9015e94f5f4a9b5a7b750

URL: 
https://github.com/llvm/llvm-project/commit/24ee6d3d3c6215db11e9015e94f5f4a9b5a7b750
DIFF: 
https://github.com/llvm/llvm-project/commit/24ee6d3d3c6215db11e9015e94f5f4a9b5a7b750.diff

LOG: [lldb][NFC] Remove unused var in SBDebugger::GetInternalVariableValue

This variable was originally just the default return value but got unused
in 6920b52be6f8731692a9bff4fe6a7b596cda00c5 .

Added: 


Modified: 
lldb/source/API/SBDebugger.cpp

Removed: 




diff  --git a/lldb/source/API/SBDebugger.cpp b/lldb/source/API/SBDebugger.cpp
index 03e6fe52969d..a854c22bb214 100644
--- a/lldb/source/API/SBDebugger.cpp
+++ b/lldb/source/API/SBDebugger.cpp
@@ -1334,7 +1334,6 @@ SBDebugger::GetInternalVariableValue(const char *var_name,
   lldb::SBStringList, SBDebugger, GetInternalVariableValue,
   (const char *, const char *), var_name, debugger_instance_name);
 
-  SBStringList ret_value;
   DebuggerSP debugger_sp(Debugger::FindDebuggerWithInstanceName(
   ConstString(debugger_instance_name)));
   Status error;



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D103391: [lldb] Add missing reproducer instrumentation to some SB classes

2021-05-31 Thread Raphael Isemann via Phabricator via lldb-commits
teemperor created this revision.
teemperor added a reviewer: JDevlieghere.
teemperor added a project: LLDB.
teemperor requested review of this revision.
Herald added a subscriber: lldb-commits.

I noticed that there might be some missing instrumentation after a related 
change popped up in D103381 .
This patch is just the result of running lldb-instr over the different parts of 
the SB API implementations (+ some manual filtering).


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D103391

Files:
  lldb/source/API/SBDebugger.cpp
  lldb/source/API/SBModuleSpec.cpp
  lldb/source/API/SBTrace.cpp


Index: lldb/source/API/SBTrace.cpp
===
--- lldb/source/API/SBTrace.cpp
+++ lldb/source/API/SBTrace.cpp
@@ -77,6 +77,9 @@
 }
 
 void SBTrace::GetTraceConfig(SBTraceOptions , SBError ) {
+  LLDB_RECORD_METHOD(void, SBTrace, GetTraceConfig,
+ (lldb::SBTraceOptions &, lldb::SBError &), options, 
error);
+
   error.SetErrorString("deprecated");
 }
 
Index: lldb/source/API/SBModuleSpec.cpp
===
--- lldb/source/API/SBModuleSpec.cpp
+++ lldb/source/API/SBModuleSpec.cpp
@@ -127,6 +127,8 @@
 }
 
 const uint8_t *SBModuleSpec::GetUUIDBytes() {
+  LLDB_RECORD_METHOD_NO_ARGS(const uint8_t *, SBModuleSpec, GetUUIDBytes);
+
   return m_opaque_up->GetUUID().GetBytes().data();
 }
 
@@ -137,6 +139,9 @@
 }
 
 bool SBModuleSpec::SetUUIDBytes(const uint8_t *uuid, size_t uuid_len) {
+  LLDB_RECORD_METHOD(bool, SBModuleSpec, SetUUIDBytes,
+ (const uint8_t *, size_t), uuid, uuid_len);
+
   m_opaque_up->GetUUID() = UUID::fromOptionalData(uuid, uuid_len);
   return m_opaque_up->GetUUID().IsValid();
 }
Index: lldb/source/API/SBDebugger.cpp
===
--- lldb/source/API/SBDebugger.cpp
+++ lldb/source/API/SBDebugger.cpp
@@ -162,6 +162,11 @@
  uint64_t ,
  uint64_t ,
  bool _debugger_specific) {
+  LLDB_RECORD_STATIC_METHOD(
+  const char *, SBDebugger, GetProgressFromEvent,
+  (const lldb::SBEvent &, uint64_t &, uint64_t &, uint64_t &, bool &),
+  event, progress_id, completed, total, is_debugger_specific);
+
   const Debugger::ProgressEventData *progress_data =
   Debugger::ProgressEventData::GetEventDataFromEvent(event.get());
   if (progress_data == nullptr)


Index: lldb/source/API/SBTrace.cpp
===
--- lldb/source/API/SBTrace.cpp
+++ lldb/source/API/SBTrace.cpp
@@ -77,6 +77,9 @@
 }
 
 void SBTrace::GetTraceConfig(SBTraceOptions , SBError ) {
+  LLDB_RECORD_METHOD(void, SBTrace, GetTraceConfig,
+ (lldb::SBTraceOptions &, lldb::SBError &), options, error);
+
   error.SetErrorString("deprecated");
 }
 
Index: lldb/source/API/SBModuleSpec.cpp
===
--- lldb/source/API/SBModuleSpec.cpp
+++ lldb/source/API/SBModuleSpec.cpp
@@ -127,6 +127,8 @@
 }
 
 const uint8_t *SBModuleSpec::GetUUIDBytes() {
+  LLDB_RECORD_METHOD_NO_ARGS(const uint8_t *, SBModuleSpec, GetUUIDBytes);
+
   return m_opaque_up->GetUUID().GetBytes().data();
 }
 
@@ -137,6 +139,9 @@
 }
 
 bool SBModuleSpec::SetUUIDBytes(const uint8_t *uuid, size_t uuid_len) {
+  LLDB_RECORD_METHOD(bool, SBModuleSpec, SetUUIDBytes,
+ (const uint8_t *, size_t), uuid, uuid_len);
+
   m_opaque_up->GetUUID() = UUID::fromOptionalData(uuid, uuid_len);
   return m_opaque_up->GetUUID().IsValid();
 }
Index: lldb/source/API/SBDebugger.cpp
===
--- lldb/source/API/SBDebugger.cpp
+++ lldb/source/API/SBDebugger.cpp
@@ -162,6 +162,11 @@
  uint64_t ,
  uint64_t ,
  bool _debugger_specific) {
+  LLDB_RECORD_STATIC_METHOD(
+  const char *, SBDebugger, GetProgressFromEvent,
+  (const lldb::SBEvent &, uint64_t &, uint64_t &, uint64_t &, bool &),
+  event, progress_id, completed, total, is_debugger_specific);
+
   const Debugger::ProgressEventData *progress_data =
   Debugger::ProgressEventData::GetEventDataFromEvent(event.get());
   if (progress_data == nullptr)
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D103390: [lldb] Remove SBCommandReturnObject::ref

2021-05-31 Thread Jan Kratochvil via Phabricator via lldb-commits
jankratochvil accepted this revision.
jankratochvil added a comment.
This revision is now accepted and ready to land.

I agree it should not be permitted to return the internal object from SB API. 
And if it still compiles after moving to `private:` then why not.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103390

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D103390: [lldb] Remove SBCommandReturnObject::ref

2021-05-31 Thread Raphael Isemann via Phabricator via lldb-commits
teemperor added a comment.

IIUC this was originally public but then we made the other users friends (but 
the public remained). But still putting this up for review in case I missed 
something


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103390

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D103390: [lldb] Remove SBCommandReturnObject::ref

2021-05-31 Thread Raphael Isemann via Phabricator via lldb-commits
teemperor created this revision.
teemperor added a reviewer: jankratochvil.
teemperor added a project: LLDB.
Herald added a subscriber: JDevlieghere.
teemperor requested review of this revision.
Herald added a subscriber: lldb-commits.

This function was added in D67589  and returns 
an internal CommandReturnObject
which isn't allowed in the SB API. This patch just makes it private as all uses
of this function are inside SBCommandReturnObject.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D103390

Files:
  lldb/include/lldb/API/SBCommandReturnObject.h


Index: lldb/include/lldb/API/SBCommandReturnObject.h
===
--- lldb/include/lldb/API/SBCommandReturnObject.h
+++ lldb/include/lldb/API/SBCommandReturnObject.h
@@ -105,9 +105,6 @@
 
   void SetError(const char *error_cstr);
 
-  // ref() is internal for LLDB only.
-  lldb_private::CommandReturnObject () const;
-
 protected:
   friend class SBCommandInterpreter;
   friend class SBOptions;
@@ -119,6 +116,8 @@
   lldb_private::CommandReturnObject *() const;
 
 private:
+  lldb_private::CommandReturnObject () const;
+
   std::unique_ptr m_opaque_up;
 };
 


Index: lldb/include/lldb/API/SBCommandReturnObject.h
===
--- lldb/include/lldb/API/SBCommandReturnObject.h
+++ lldb/include/lldb/API/SBCommandReturnObject.h
@@ -105,9 +105,6 @@
 
   void SetError(const char *error_cstr);
 
-  // ref() is internal for LLDB only.
-  lldb_private::CommandReturnObject () const;
-
 protected:
   friend class SBCommandInterpreter;
   friend class SBOptions;
@@ -119,6 +116,8 @@
   lldb_private::CommandReturnObject *() const;
 
 private:
+  lldb_private::CommandReturnObject () const;
+
   std::unique_ptr m_opaque_up;
 };
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits