| Message ID | 20240322131451.3092931-2-dan.scally@ideasonboard.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
Hi Dan, thank you for the patch. On Fri, Mar 22, 2024 at 01:14:42PM +0000, Daniel Scally wrote: > For convenience's sake allow the creation of empty Histograms so > they can be embedded within other Classes and filled out with > data at some later point in time. > > Signed-off-by: Daniel Scally <dan.scally@ideasonboard.com> > --- > src/ipa/libipa/histogram.h | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/src/ipa/libipa/histogram.h b/src/ipa/libipa/histogram.h > index 05bb4b80..db13c155 100644 > --- a/src/ipa/libipa/histogram.h > +++ b/src/ipa/libipa/histogram.h > @@ -22,6 +22,7 @@ namespace ipa { > class Histogram > { > public: > + Histogram() { cumulative_.push_back(0); }; The second semicolon is not necessary (CI will check that). With that fixed: Reviewed-by: Stefan Klug <stefan.klug@ideasonboard.com> Cheers Stefan > Histogram(Span<const uint32_t> data); > size_t bins() const { return cumulative_.size() - 1; } > uint64_t total() const { return cumulative_[cumulative_.size() - 1]; } > -- > 2.34.1 >
Hi Dan On Fri, Mar 22, 2024 at 01:14:42PM +0000, Daniel Scally wrote: > For convenience's sake allow the creation of empty Histograms so > they can be embedded within other Classes and filled out with > data at some later point in time. > How are you going to add data to the Histogram since they seem to be expected to be passed in at construction time ? > Signed-off-by: Daniel Scally <dan.scally@ideasonboard.com> > --- > src/ipa/libipa/histogram.h | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/src/ipa/libipa/histogram.h b/src/ipa/libipa/histogram.h > index 05bb4b80..db13c155 100644 > --- a/src/ipa/libipa/histogram.h > +++ b/src/ipa/libipa/histogram.h > @@ -22,6 +22,7 @@ namespace ipa { > class Histogram > { > public: > + Histogram() { cumulative_.push_back(0); }; > Histogram(Span<const uint32_t> data); > size_t bins() const { return cumulative_.size() - 1; } > uint64_t total() const { return cumulative_[cumulative_.size() - 1]; } > -- > 2.34.1 >
Hi Jacopo On 25/03/2024 15:45, Jacopo Mondi wrote: > Hi Dan > > On Fri, Mar 22, 2024 at 01:14:42PM +0000, Daniel Scally wrote: >> For convenience's sake allow the creation of empty Histograms so >> they can be embedded within other Classes and filled out with >> data at some later point in time. >> > How are you going to add data to the Histogram since they seem to be > expected to be passed in at construction time ? See for example patch #8, with: + hist_ = Histogram(Span<const uint32_t>(params->hist.hist_bins, + context.hw->numHistogramBins)); > >> Signed-off-by: Daniel Scally <dan.scally@ideasonboard.com> >> --- >> src/ipa/libipa/histogram.h | 1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/src/ipa/libipa/histogram.h b/src/ipa/libipa/histogram.h >> index 05bb4b80..db13c155 100644 >> --- a/src/ipa/libipa/histogram.h >> +++ b/src/ipa/libipa/histogram.h >> @@ -22,6 +22,7 @@ namespace ipa { >> class Histogram >> { >> public: >> + Histogram() { cumulative_.push_back(0); }; >> Histogram(Span<const uint32_t> data); >> size_t bins() const { return cumulative_.size() - 1; } >> uint64_t total() const { return cumulative_[cumulative_.size() - 1]; } >> -- >> 2.34.1 >>
Hi Dan, Thank you for the patch. On Mon, Mar 25, 2024 at 06:42:32PM +0000, Daniel Scally wrote: > On 25/03/2024 15:45, Jacopo Mondi wrote: > > On Fri, Mar 22, 2024 at 01:14:42PM +0000, Daniel Scally wrote: > >> For convenience's sake allow the creation of empty Histograms so > >> they can be embedded within other Classes and filled out with > >> data at some later point in time. > >> > > How are you going to add data to the Histogram since they seem to be > > expected to be passed in at construction time ? > > See for example patch #8, with: > > + hist_ = Histogram(Span<const uint32_t>(params->hist.hist_bins, + context.hw->numHistogramBins)); > > >> Signed-off-by: Daniel Scally <dan.scally@ideasonboard.com> > >> --- > >> src/ipa/libipa/histogram.h | 1 + > >> 1 file changed, 1 insertion(+) > >> > >> diff --git a/src/ipa/libipa/histogram.h b/src/ipa/libipa/histogram.h > >> index 05bb4b80..db13c155 100644 > >> --- a/src/ipa/libipa/histogram.h > >> +++ b/src/ipa/libipa/histogram.h > >> @@ -22,6 +22,7 @@ namespace ipa { > >> class Histogram > >> { > >> public: > >> + Histogram() { cumulative_.push_back(0); }; Apart from the missing documentation, this looks OK. > >> Histogram(Span<const uint32_t> data); > >> size_t bins() const { return cumulative_.size() - 1; } > >> uint64_t total() const { return cumulative_[cumulative_.size() - 1]; }
Hi Dan, On Fri, Mar 22, 2024 at 01:14:42PM +0000, Daniel Scally wrote: > For convenience's sake allow the creation of empty Histograms so > they can be embedded within other Classes and filled out with > data at some later point in time. > > Signed-off-by: Daniel Scally <dan.scally@ideasonboard.com> Reviewed-by: Paul Elder <paul.elder@ideasonboard.com> > --- > src/ipa/libipa/histogram.h | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/src/ipa/libipa/histogram.h b/src/ipa/libipa/histogram.h > index 05bb4b80..db13c155 100644 > --- a/src/ipa/libipa/histogram.h > +++ b/src/ipa/libipa/histogram.h > @@ -22,6 +22,7 @@ namespace ipa { > class Histogram > { > public: > + Histogram() { cumulative_.push_back(0); }; > Histogram(Span<const uint32_t> data); > size_t bins() const { return cumulative_.size() - 1; } > uint64_t total() const { return cumulative_[cumulative_.size() - 1]; } > -- > 2.34.1 >
diff --git a/src/ipa/libipa/histogram.h b/src/ipa/libipa/histogram.h index 05bb4b80..db13c155 100644 --- a/src/ipa/libipa/histogram.h +++ b/src/ipa/libipa/histogram.h @@ -22,6 +22,7 @@ namespace ipa { class Histogram { public: + Histogram() { cumulative_.push_back(0); }; Histogram(Span<const uint32_t> data); size_t bins() const { return cumulative_.size() - 1; } uint64_t total() const { return cumulative_[cumulative_.size() - 1]; }
For convenience's sake allow the creation of empty Histograms so they can be embedded within other Classes and filled out with data at some later point in time. Signed-off-by: Daniel Scally <dan.scally@ideasonboard.com> --- src/ipa/libipa/histogram.h | 1 + 1 file changed, 1 insertion(+)