[PATCH v3 5/6] Optimize default filtering

[Eugene Syromiatnikov]

On Thu, Jun 29, 2017 at 02:46:14PM +0700, Nikolay Marchuk wrote:
> * filter_action.c (default_flags, clear_default_flags): Add default flags.
> (add_action): Use action type as argument. Clear default flags.
> (filter_syscall): Add default_flags to qual_flg.
> * strace.c (init): Remove default filters.
There's also another change, change in the add_action function
signature, and the presence of this change here (and its non-incorporation
into the initial "Introduce new filtering architecture" patch) is not justified.

> ---
>  filter_action.c | 30 ++++++++++++++++++++++++------
>  strace.c        |  3 ---
>  2 files changed, 24 insertions(+), 9 deletions(-)
> 
> diff --git a/filter_action.c b/filter_action.c
> index fd3fa92..0c235da 100644
> --- a/filter_action.c
> +++ b/filter_action.c
> @@ -88,6 +88,7 @@ struct filter_action {
>  	void* _priv_data;
>  };
>  
> +static int default_flags = DEFAULT_QUAL_FLAGS;
>  static struct filter_action *filter_actions;
>  static unsigned int nfilter_actions;
>  
> @@ -103,10 +104,28 @@ compare_action_priority(const void *a, const void *b)
>  void
>  sort_filter_actions(void)
>  {
> +	if (nfilter_actions == 0)
> +		return;
I'm pretty sure qsort exits rather early when an empty array is provided,
does this additional check actually bring any speed benefit for the one-time
action?

>  	qsort(filter_actions, nfilter_actions, sizeof(struct filter_action),
>  	      &compare_action_priority);
>  }
>  
> +static void
> +clear_default_flags(const char *name)
> +{
> +	if ((default_flags & QUAL_TRACE) && !strcmp(name, "trace")) {
> +		default_flags &= ~QUAL_TRACE;
> +		return;
> +	} else if ((default_flags & QUAL_ABBREV) && !strcmp(name, "abbrev")) {
> +		default_flags &= ~QUAL_ABBREV;
> +		return;
> +	} else if ((default_flags & QUAL_VERBOSE) && !strcmp(name, "verbose")) {
> +		default_flags &= ~QUAL_VERBOSE;
> +		return;
> +	}
What still bothers me is that one need strcmp in order to compare action
type IDs (which are not generated dynamically, but rather
statically-defined).

> +
Excess empty line.

> +}
> +
>  static const struct filter_action_type *
>  lookup_filter_action_type(const char *str)
>  {
> @@ -122,11 +141,9 @@ lookup_filter_action_type(const char *str)
>  }
>  
>  static struct filter_action *
> -add_action(const char *name)
> +add_action(const struct filter_action_type *type)
>  {
> -	const struct filter_action_type *type = lookup_filter_action_type(name);
> -	if (!type)
> -		error_msg_and_die("invalid filter action '%s'", name);
> +	clear_default_flags(type->name);
>  	filter_actions = xreallocarray(filter_actions, ++nfilter_actions,
>  	                               sizeof(struct filter_action));
>  	struct filter_action *action = &filter_actions[nfilter_actions - 1];
> @@ -143,13 +160,13 @@ find_or_add_action(const char *name)
>  	if (!type)
>  		error_msg_and_die("invalid filter action '%s'", name);
>  	if (type->parse_args != &parse_null)
> -		return add_action(name);
> +		return add_action(type);
>  	unsigned int i;
>  	for (i = 0; i < nfilter_actions; i++) {
>  		if (filter_actions[i].type == type)
>  			return &filter_actions[i];
>  	}
> -	return add_action(name);
> +	return add_action(type);
>  }
>  
>  static void
> @@ -186,6 +203,7 @@ set_qualify_mode(struct filter_action *action)
>  void
>  filter_syscall(struct tcb *tcp)
>  {
> +	tcp->qual_flg |= default_flags;
>  	unsigned int i;
>  	for (i = 0; i < nfilter_actions; i++) {
>  		run_filter_action(tcp, &filter_actions[i]);
> diff --git a/strace.c b/strace.c
> index e48ed76..0733b84 100644
> --- a/strace.c
> +++ b/strace.c
> @@ -1622,9 +1622,6 @@ init(int argc, char *argv[])
>  	shared_log = stderr;
>  	set_sortby(DEFAULT_SORTBY);
>  	set_personality(DEFAULT_PERSONALITY);
> -	parse_qualify_filter("trace=all");
> -	parse_qualify_filter("abbrev=all");
> -	parse_qualify_filter("verbose=all");
>  #if DEFAULT_QUAL_FLAGS != (QUAL_TRACE | QUAL_ABBREV | QUAL_VERBOSE)
>  # error Bug in DEFAULT_QUAL_FLAGS
>  #endif
> -- 
> 2.1.4