On Wed, 11 Aug 2021 15:12:02 +0300
"Yordan Karadzhov (VMware)" <y.karadz@xxxxxxxxx> wrote:
> The current implementation of tep_print_fields() loops over all
> individual fields of the event and calls tep_print_field() for
> each one. In the same time inside tep_print_field() we loop over
> the list of all tokens of the printing format descriptor in order
> to determine how the current field must be printed (its own
> appropriate printing format). The problem is that in this second
> loop over the tokens we always start from the very first token
> and this can be quite inefficient for example in a case of a
> kprobe that has a large number of fields. This patch optimizes
> tep_print_fields(), allowing the traverse of the list of tokens
> to continue from the place reached when we searched for the
> format of the previous field.
>
> Signed-off-by: Yordan Karadzhov (VMware) <y.karadz@xxxxxxxxx>
> ---
> src/event-parse.c | 30 +++++++++++++++++++++++-------
> 1 file changed, 23 insertions(+), 7 deletions(-)
>
> diff --git a/src/event-parse.c b/src/event-parse.c
> index 9399555..7831e74 100644
> --- a/src/event-parse.c
> +++ b/src/event-parse.c
> @@ -5392,14 +5392,15 @@ static void tep_print_field_raw(struct trace_seq *s, void *data,
> static int print_parse_data(struct tep_print_parse *parse, struct trace_seq *s,
> void *data, int size, struct tep_event *event);
>
> -void tep_print_field(struct trace_seq *s, void *data,
> - struct tep_format_field *field)
> +void static inline _tep_print_field(struct trace_seq *s, void *data,
> + struct tep_format_field *field,
> + struct tep_print_parse **parse_ptr)
> {
> struct tep_event *event = field->event;
> struct tep_print_parse *parse;
> bool has_0x;
>
> - parse = event->print_fmt.print_cache;
> + parse = parse_ptr ? *parse_ptr : event->print_fmt.print_cache;
>
> if (event->flags & TEP_EVENT_FL_FAILED)
> goto out;
> @@ -5407,7 +5408,7 @@ void tep_print_field(struct trace_seq *s, void *data,
> if (field->flags & (TEP_FIELD_IS_ARRAY || TEP_FIELD_IS_STRING))
> goto out;
>
> - for (;parse; parse = parse->next) {
> + while (parse) {
If the parse field is not found, this will loop forever. We need this:
struct tep_print_parse *start_parse;
start_parse = parse;
do {
> if (parse->type == PRINT_FMT_STRING) {
> int len = strlen(parse->format);
>
> @@ -5417,21 +5418,29 @@ void tep_print_field(struct trace_seq *s, void *data,
> else
> has_0x = false;
>
> - continue;
> + goto next;
> }
>
> if (!parse->arg ||
> parse->arg->type != TEP_PRINT_FIELD ||
> parse->arg->field.field != field) {
> has_0x = false;
> - continue;
> + goto next;
> }
>
> if (has_0x)
> trace_seq_puts(s, "0x");
>
> print_parse_data(parse, s, data, field->size, event);
> +
> + if (parse_ptr)
> + *parse_ptr = parse->next;
> +
> return;
> +
> + next:
> + parse = parse->next ? parse->next :
> + event->print_fmt.print_cache;
} while (parse != start_parse);
-- Steve
> }
>
> out:
> @@ -5439,15 +5448,22 @@ void tep_print_field(struct trace_seq *s, void *data,
> tep_print_field_raw(s, data, field);
> }
>
> +void tep_print_field(struct trace_seq *s, void *data,
> + struct tep_format_field *field)
> +{
> + _tep_print_field(s, data, field, NULL);
> +}
> +
> void tep_print_fields(struct trace_seq *s, void *data,
> int size __maybe_unused, struct tep_event *event)
> {
> + struct tep_print_parse *parse = event->print_fmt.print_cache;
> struct tep_format_field *field;
>
> field = event->format.fields;
> while (field) {
> trace_seq_printf(s, " %s=", field->name);
> - tep_print_field(s, data, field);
> + _tep_print_field(s, data, field, &parse);
> field = field->next;
> }
> }
|