Re: [PATCH v4 10/53] perf record: Be lazier in allocating lost samples buffer

[Arnaldo Carvalho de Melo]

Em Thu, Nov 02, 2023 at 10:56:52AM -0700, Ian Rogers escreveu:
> Wait until a lost sample occurs to allocate the lost samples buffer,
> often the buffer isn't necessary. This saves a 64kb allocation and
> 5.3kb of peak memory consumption.
> 
> Signed-off-by: Ian Rogers <irogers@xxxxxxxxxx>
> ---
>  tools/perf/builtin-record.c | 29 +++++++++++++++++++----------
>  1 file changed, 19 insertions(+), 10 deletions(-)
> 
> diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
> index 9b4f3805ca92..b6c8c1371b39 100644
> --- a/tools/perf/builtin-record.c
> +++ b/tools/perf/builtin-record.c
> @@ -1924,21 +1924,13 @@ static void __record__save_lost_samples(struct record *rec, struct evsel *evsel,
>  static void record__read_lost_samples(struct record *rec)
>  {
>  	struct perf_session *session = rec->session;
> -	struct perf_record_lost_samples *lost;
> +	struct perf_record_lost_samples *lost = NULL;
>  	struct evsel *evsel;
>  
>  	/* there was an error during record__open */
>  	if (session->evlist == NULL)
>  		return;
>  
> -	lost = zalloc(PERF_SAMPLE_MAX_SIZE);
> -	if (lost == NULL) {
> -		pr_debug("Memory allocation failed\n");
> -		return;
> -	}

Shouldn't we take the time here and instead improve this error message
and then propagate the error?

For instance, we may want to still get some perf.data file without these
records but inform the user at this point how many records were lost
(count.lost)?

- Arnaldo

> -
> -	lost->header.type = PERF_RECORD_LOST_SAMPLES;
> -
>  	evlist__for_each_entry(session->evlist, evsel) {
>  		struct xyarray *xy = evsel->core.sample_id;
>  		u64 lost_count;
> @@ -1961,6 +1953,14 @@ static void record__read_lost_samples(struct record *rec)
>  				}
>  
>  				if (count.lost) {
> +					if (!lost) {
> +						lost = zalloc(PERF_SAMPLE_MAX_SIZE);
> +						if (!lost) {
> +							pr_debug("Memory allocation failed\n");
> +							return;
> +						}
> +						lost->header.type = PERF_RECORD_LOST_SAMPLES;
> +					}
>  					__record__save_lost_samples(rec, evsel, lost,
>  								    x, y, count.lost, 0);
>  				}
> @@ -1968,9 +1968,18 @@ static void record__read_lost_samples(struct record *rec)
>  		}
>  
>  		lost_count = perf_bpf_filter__lost_count(evsel);
> -		if (lost_count)
> +		if (lost_count) {
> +			if (!lost) {
> +				lost = zalloc(PERF_SAMPLE_MAX_SIZE);
> +				if (!lost) {
> +					pr_debug("Memory allocation failed\n");
> +					return;
> +				}
> +				lost->header.type = PERF_RECORD_LOST_SAMPLES;
> +			}
>  			__record__save_lost_samples(rec, evsel, lost, 0, 0, lost_count,
>  						    PERF_RECORD_MISC_LOST_SAMPLES_BPF);
> +		}
>  	}
>  out:
>  	free(lost);
> -- 
> 2.42.0.869.gea05f2083d-goog
> 

-- 

- Arnaldo