https://github.com/chelcassanova closed
https://github.com/llvm/llvm-project/pull/81026
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
@@ -12,6 +12,7 @@
#include "lldb/Utility/ConstString.h"
#include "lldb/lldb-types.h"
#include
+#include
chelcassanova wrote:
I shouldn't need a sorted map so I'll go with StringMap for the updated patch
with the class that will handle the progress report
@@ -99,6 +105,10 @@ class Progress {
private:
void ReportProgress();
static std::atomic g_id;
+ static std::atomic g_refcount;
+ /// Map that tracks each progress object and if we've seen its start and stop
+ /// events
+ static std::unordered_map g_map;
@@ -12,6 +12,7 @@
#include "lldb/Utility/ConstString.h"
#include "lldb/lldb-types.h"
#include
+#include
adrian-prantl wrote:
Unless you need the sorted iteration property you probably want to use
something more efficient instead.
@@ -99,6 +105,10 @@ class Progress {
private:
void ReportProgress();
static std::atomic g_id;
+ static std::atomic g_refcount;
+ /// Map that tracks each progress object and if we've seen its start and stop
+ /// events
+ static std::unordered_map g_map;
clayborg wrote:
> The discussions happening here are talking about 2 major things, how to do
> the bookkeeping of the map that keeps track of progress reports and where to
> do that bookkeeping. I think it makes sense to split up this work into
> smaller patches as such:
>
> 1. Since it's
chelcassanova wrote:
The discussions happening here are talking about 2 major things, how to do the
bookkeeping of the map that keeps track of progress reports and where to do
that bookkeeping. I think it makes sense to split up this work into smaller
patches as such:
1. Since it's best to
chelcassanova wrote:
I'll start with this by changing this so that bookkeeping is done with the new
bit instead of being done in the constructor for `Progress`.
https://github.com/llvm/llvm-project/pull/81026
___
lldb-commits mailing list
JDevlieghere wrote:
> > My understanding is that the constructor conveys whether something is an
> > "aggregate" progress event or not and they're broadcast differently so that
> > the listener can decide how they want to receive these "aggregate" events.
>
> My idea is the user decides how
clayborg wrote:
> > A few questions I have:
> >
> > * do we really want each progress to select if it should be coalesced as a
> > `Progress` constructor arguments? Or do we want a global setting on how
> > progress events should be delivered?
>
> My understanding is that the constructor
JDevlieghere wrote:
> A few questions I have:
>
> * do we really want each progress to select if it should be coalesced as a
> `Progress` constructor arguments? Or do we want a global setting on how
> progress events should be delivered?
My understanding is that the constructor conveys
@@ -99,6 +105,10 @@ class Progress {
private:
void ReportProgress();
static std::atomic g_id;
+ static std::atomic g_refcount;
+ /// Map that tracks each progress object and if we've seen its start and stop
+ /// events
+ static std::unordered_map g_map;
@@ -99,6 +105,10 @@ class Progress {
private:
void ReportProgress();
static std::atomic g_id;
+ static std::atomic g_refcount;
+ /// Map that tracks each progress object and if we've seen its start and stop
+ /// events
+ static std::unordered_map g_map;
@@ -9,26 +9,34 @@
#include "lldb/Core/Progress.h"
#include "lldb/Core/Debugger.h"
-#include "lldb/Utility/StreamString.h"
#include
using namespace lldb;
using namespace lldb_private;
std::atomic Progress::g_id(0);
+std::atomic Progress::g_refcount(1);
@@ -9,26 +9,34 @@
#include "lldb/Core/Progress.h"
#include "lldb/Core/Debugger.h"
-#include "lldb/Utility/StreamString.h"
#include
using namespace lldb;
using namespace lldb_private;
std::atomic Progress::g_id(0);
+std::atomic Progress::g_refcount(1);
@@ -99,6 +105,10 @@ class Progress {
private:
void ReportProgress();
static std::atomic g_id;
+ static std::atomic g_refcount;
+ /// Map that tracks each progress object and if we've seen its start and stop
+ /// events
+ static std::unordered_map g_map;
@@ -99,6 +105,10 @@ class Progress {
private:
void ReportProgress();
static std::atomic g_id;
+ static std::atomic g_refcount;
+ /// Map that tracks each progress object and if we've seen its start and stop
+ /// events
+ static std::unordered_map g_map;
@@ -99,6 +105,10 @@ class Progress {
private:
void ReportProgress();
static std::atomic g_id;
+ static std::atomic g_refcount;
+ /// Map that tracks each progress object and if we've seen its start and stop
+ /// events
+ static std::unordered_map g_map;
@@ -9,26 +9,34 @@
#include "lldb/Core/Progress.h"
#include "lldb/Core/Debugger.h"
-#include "lldb/Utility/StreamString.h"
#include
using namespace lldb;
using namespace lldb_private;
std::atomic Progress::g_id(0);
+std::atomic Progress::g_refcount(1);
@@ -117,6 +127,7 @@ class Progress {
/// to ensure that we don't send progress updates after progress has
/// completed.
bool m_complete = false;
+ bool m_type;
chelcassanova wrote:
Holdover from when I used a bool for this value before switching an
https://github.com/medismailben edited
https://github.com/llvm/llvm-project/pull/81026
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
https://github.com/medismailben edited
https://github.com/llvm/llvm-project/pull/81026
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
https://github.com/medismailben edited
https://github.com/llvm/llvm-project/pull/81026
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
@@ -9,26 +9,34 @@
#include "lldb/Core/Progress.h"
#include "lldb/Core/Debugger.h"
-#include "lldb/Utility/StreamString.h"
#include
using namespace lldb;
using namespace lldb_private;
std::atomic Progress::g_id(0);
+std::atomic Progress::g_refcount(1);
@@ -99,6 +105,10 @@ class Progress {
private:
void ReportProgress();
static std::atomic g_id;
+ static std::atomic g_refcount;
medismailben wrote:
By making the refcount static, it's shared across all the `Progress` instances,
but IIUC, you want this
https://github.com/medismailben edited
https://github.com/llvm/llvm-project/pull/81026
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
https://github.com/medismailben requested changes to this pull request.
https://github.com/llvm/llvm-project/pull/81026
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
https://github.com/clayborg edited
https://github.com/llvm/llvm-project/pull/81026
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
@@ -9,26 +9,34 @@
#include "lldb/Core/Progress.h"
#include "lldb/Core/Debugger.h"
-#include "lldb/Utility/StreamString.h"
#include
using namespace lldb;
using namespace lldb_private;
std::atomic Progress::g_id(0);
+std::atomic Progress::g_refcount(1);
@@ -9,26 +9,34 @@
#include "lldb/Core/Progress.h"
#include "lldb/Core/Debugger.h"
-#include "lldb/Utility/StreamString.h"
#include
using namespace lldb;
using namespace lldb_private;
std::atomic Progress::g_id(0);
+std::atomic Progress::g_refcount(1);
@@ -55,6 +56,10 @@ namespace lldb_private {
class Progress {
public:
+ enum {
+eProgressLinearReports,
+eProgressCoalecseReports,
+ };
clayborg wrote:
Should we add a new setting for this? Something like:
```
(lldb) settings set progress-report
@@ -38,6 +46,13 @@ Progress::~Progress() {
std::lock_guard guard(m_mutex);
if (!m_completed)
m_completed = m_total;
+
+ if (m_type == Progress::eProgressCoalecseReports) {
+--g_map.at(m_title);
+if (g_map.at(m_title) == 0)
+ g_map.erase(m_title);
https://github.com/clayborg commented:
So I added comments on fixes to the current implementation in inline comments.
A few questions I have:
- do we really want each progress to select if it should be coalesced as a
`Progress` constructor arguments? Or do we want a global setting on how
@@ -9,26 +9,34 @@
#include "lldb/Core/Progress.h"
#include "lldb/Core/Debugger.h"
-#include "lldb/Utility/StreamString.h"
#include
using namespace lldb;
using namespace lldb_private;
std::atomic Progress::g_id(0);
+std::atomic Progress::g_refcount(1);
@@ -55,6 +56,10 @@ namespace lldb_private {
class Progress {
public:
+ enum {
+eProgressLinearReports,
+eProgressCoalecseReports,
medismailben wrote:
```suggestion
eProgressCoalesceReports,
```
https://github.com/llvm/llvm-project/pull/81026
@@ -9,26 +9,34 @@
#include "lldb/Core/Progress.h"
#include "lldb/Core/Debugger.h"
-#include "lldb/Utility/StreamString.h"
#include
using namespace lldb;
using namespace lldb_private;
std::atomic Progress::g_id(0);
+std::atomic Progress::g_refcount(1);
@@ -99,6 +105,10 @@ class Progress {
private:
void ReportProgress();
static std::atomic g_id;
+ static std::atomic g_refcount;
+ /// Map that tracks each progress object and if we've seen its start and stop
+ /// events
+ static std::unordered_map g_map;
@@ -117,6 +127,7 @@ class Progress {
/// to ensure that we don't send progress updates after progress has
/// completed.
bool m_complete = false;
+ bool m_type;
medismailben wrote:
This is confusing, why would you store your enum value in a `bool`
https://github.com/medismailben requested changes to this pull request.
I think the progress map should be held by the debugger instance, not a static
member of the Progress class, since some progress reports could be targeting a
specific debugger.
@@ -55,6 +56,10 @@ namespace lldb_private {
class Progress {
public:
+ enum {
+eProgressLinearReports,
medismailben wrote:
Please comment what each enum value means
https://github.com/llvm/llvm-project/pull/81026
https://github.com/medismailben edited
https://github.com/llvm/llvm-project/pull/81026
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
https://github.com/chelcassanova updated
https://github.com/llvm/llvm-project/pull/81026
>From a80637fe2471c3f1adc2b353cef41887bcd55a3c Mon Sep 17 00:00:00 2001
From: Chelsea Cassanova
Date: Tue, 6 Feb 2024 10:48:39 -0800
Subject: [PATCH] [lldb][progress][NFC] Add groundwork to keep track of
github-actions[bot] wrote:
:warning: C/C++ code formatter, clang-format found issues in your code.
:warning:
You can test this locally with the following command:
``bash
git-clang-format --diff a8ab8306069e8e53b5148ceec7624d7d36ffb459
d10c1c494972429eb5c42ccb8a1dfcbff5d7bfcd --
llvmbot wrote:
@llvm/pr-subscribers-lldb
Author: Chelsea Cassanova (chelcassanova)
Changes
As part of the effort to improve progress reporting in LLDB
(https://discourse.llvm.org/t/rfc-improve-lldb-progress-reporting/75717) we
want a way to keep track of progress reports to see if
https://github.com/chelcassanova created
https://github.com/llvm/llvm-project/pull/81026
As part of the effort to improve progress reporting in LLDB
(https://discourse.llvm.org/t/rfc-improve-lldb-progress-reporting/75717) we
want a way to keep track of progress reports to see if they're
45 matches
Mail list logo
