Re: [PATCH v11 19/24] perf session: Introduce reader objects in session object

[Jiri Olsa]

On Tue, Aug 17, 2021 at 11:23:22AM +0300, Alexey Bayduraev wrote:
> Allow to allocate multiple reader objects, so we could load multiple
> data files located in data directory at the same time.
> 
> Design and implementation are based on the prototype [1], [2].
> 
> [1] git clone https://git.kernel.org/pub/scm/linux/kernel/git/jolsa/perf.git -b perf/record_threads
> [2] https://lore.kernel.org/lkml/20180913125450.21342-1-jolsa@xxxxxxxxxx/
> 
> Suggested-by: Jiri Olsa <jolsa@xxxxxxxxxx>
> Acked-by: Namhyung Kim <namhyung@xxxxxxxxx>
> Reviewed-by: Riccardo Mancini <rickyman7@xxxxxxxxx>
> Tested-by: Riccardo Mancini <rickyman7@xxxxxxxxx>
> Signed-off-by: Alexey Bayduraev <alexey.v.bayduraev@xxxxxxxxxxxxxxx>
> ---
>  tools/perf/util/session.c | 33 +++++++++++++++++++++------------
>  tools/perf/util/session.h |  3 +++
>  2 files changed, 24 insertions(+), 12 deletions(-)
> 
> diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
> index a82204d4c4d6..1f92d3cfcb1d 100644
> --- a/tools/perf/util/session.c
> +++ b/tools/perf/util/session.c
> @@ -343,6 +343,10 @@ void perf_session__delete(struct perf_session *session)
>  	auxtrace_index__free(&session->auxtrace_index);
>  	perf_session__destroy_kernel_maps(session);
>  	perf_session__delete_threads(session);
> +	if (session->readers) {
> +		zfree(&session->readers);
> +		session->nr_readers = 0;
> +	}
>  	perf_session__release_decomp_events(session);
>  	perf_env__exit(&session->header.env);
>  	machines__exit(&session->machines);
> @@ -2303,14 +2307,7 @@ static s64 process_simple(struct perf_session *session,
>  
>  static int __perf_session__process_events(struct perf_session *session)
>  {
> -	struct reader rd = {
> -		.fd		= perf_data__fd(session->data),
> -		.data_size	= session->header.data_size,
> -		.data_offset	= session->header.data_offset,
> -		.process	= process_simple,
> -		.path		= session->data->file.path,
> -		.in_place_update = session->data->in_place_update,
> -	};
> +	struct reader *rd;
>  	struct ordered_events *oe = &session->ordered_events;
>  	struct perf_tool *tool = session->tool;
>  	struct ui_progress prog;
> @@ -2318,12 +2315,24 @@ static int __perf_session__process_events(struct perf_session *session)
>  
>  	perf_tool__fill_defaults(tool);
>  
> -	if (rd.data_size == 0)
> -		return -1;
> +	rd = session->readers = zalloc(sizeof(struct reader));
> +	if (!rd)
> +		return -ENOMEM;
> +
> +	session->nr_readers = 1;
> +
> +	*rd = (struct reader) {
> +		.fd		 = perf_data__fd(session->data),
> +		.data_size	 = session->header.data_size,
> +		.data_offset	 = session->header.data_offset,
> +		.process	 = process_simple,
> +		.path		 = session->data->file.path,
> +		.in_place_update = session->data->in_place_update,
> +	};
>  
> -	ui_progress__init_size(&prog, rd.data_size, "Processing events...");
> +	ui_progress__init_size(&prog, rd->data_size, "Processing events...");
>  
> -	err = reader__process_events(&rd, session, &prog);
> +	err = reader__process_events(rd, session, &prog);
>  	if (err)
>  		goto out_err;

hm, leaking rd in here?

jirka

>  	/* do the final flush for ordered samples */
> diff --git a/tools/perf/util/session.h b/tools/perf/util/session.h
> index 308cf48e0945..b9b9468d8f17 100644
> --- a/tools/perf/util/session.h
> +++ b/tools/perf/util/session.h
> @@ -19,6 +19,7 @@ struct thread;
>  
>  struct auxtrace;
>  struct itrace_synth_opts;
> +struct reader;
>  
>  struct perf_session {
>  	struct perf_header	header;
> @@ -41,6 +42,8 @@ struct perf_session {
>  	struct zstd_data	zstd_data;
>  	struct decomp		*decomp;
>  	struct decomp		*decomp_last;
> +	struct reader		*readers;
> +	int			nr_readers;
>  };
>  
>  struct decomp {
> -- 
> 2.19.0
>