[Pixman] [PATCH 4/8] perfstat: Add performance statistics analyzer

[Søren Sandmann]

Taekyun Kim <podain77 at gmail.com> writes:

> diff --git a/pixman/pixman-perf-stat.c b/pixman/pixman-perf-stat.c
> new file mode 100644
> index 0000000..2986777
> --- /dev/null
> +++ b/pixman/pixman-perf-stat.c
> @@ -0,0 +1,541 @@
> +/*
> + * Copyright © 2011 SCore Corporation
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the next
> + * paragraph) shall be included in all copies or substantial portions of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
> + * DEALINGS IN THE SOFTWARE.
> + *
> + * Author:  Siarhei Siamashka (siarhei.siamashka at nokia.com)
> + * Author:  Taekyun Kim (podain77 at gmail.com)
> + */

Does Siarhei and/or Nokia own any copyright on this code? 

> +/* Performance statistics analyzer.
> + * This tool accumulate performance data for each composite path and
> + * report the result at the end of the program.
> + *
> + * TODO: Defense against tool chain which does not support attribute constructor
> + * TODO: Use debug channel rather than stdout
> + */
> +
> +#ifdef HAVE_CONFIG_H
> +#include <config.h>
> +#endif
> +
> +#include "pixman-private.h"
> +
> +#include <stdio.h>
> +#include <stdlib.h>
> +
> +#ifdef HAVE_GETTIMEOFDAY
> +#include <sys/time.h>
> +#else
> +#include <time.h>
> +#endif
> +
> +#define PERFSTAT_MAX_ENTRIES	1024
> +#define PERFSTAT_LOG_CHANNEL	stdout

Maybe stderr would be better. In the X server stderr ends up in the log
file, whereas stdout is discarded.

> +void
> +perfstat_add_composite (pixman_implementation_type_t	imp_type,
> +			pixman_op_t			op,
> +			pixman_format_code_t		src,
> +			uint32_t			src_flags,
> +			pixman_format_code_t		mask,
> +			uint32_t			mask_flags,
> +			pixman_format_code_t		dest,
> +			uint32_t			dest_flags,
> +			int32_t				width,
> +			int32_t				height,
> +			double				elapsed)
> +{
> +    int i;
> +
> +    if (src == PIXMAN_unknown || mask == PIXMAN_unknown)
> +	return;
> +
> +    if (!enable_perfstat)
> +	return;
> +
> +    PIXMAN_MUTEX_LOCK (perfstat_mutex);
> +
> +    for (i = 0; i < perfstat_entry_count; ++i)
> +    {
> +	if (perfstat[i].type == PERFSTAT_COMPOSITE &&
> +	    perfstat[i].imp_type == imp_type &&
> +	    perfstat[i].api.composite.op == op &&
> +	    perfstat[i].api.composite.src == src &&
> +	    perfstat[i].api.composite.src_flags == src_flags &&
> +	    perfstat[i].api.composite.mask == mask &&
> +	    perfstat[i].api.composite.mask_flags == mask_flags &&
> +	    perfstat[i].api.composite.dest == dest &&
> +	    perfstat[i].api.composite.dest_flags == dest_flags)
> +	{
> +	    perfstat[i].images_count++;
> +	    perfstat[i].scanlines_count += height;
> +	    perfstat[i].pixels_count += (uint64_t)width * height;
> +	    perfstat[i].time += elapsed;
> +
> +	    PIXMAN_MUTEX_UNLOCK (perfstat_mutex);
> +	    return;
> +	}

There are various cases of this UNLOCK()/return. You could save a bit of
code by simply doing "goto out", with an out label before the final UNLOCK.

> +static const char*
> +get_op_string (pixman_op_t op)
> +{
> +    switch (op)
> +    {
> +    case PIXMAN_OP_CLEAR:
> +	return "   clear";
> +    case PIXMAN_OP_SRC:
> +	return "     src";
> +    case PIXMAN_OP_DST:
> +	return "     dst";
> +    case PIXMAN_OP_OVER:
> +	return "    over";
> +    case PIXMAN_OP_OVER_REVERSE:
> +	return "over_rev";
> +    case PIXMAN_OP_IN:
> +	return "      in";
> +    case PIXMAN_OP_IN_REVERSE:
> +	return "  in_rev";
> +    case PIXMAN_OP_OUT:
> +	return "     out";
> +    case PIXMAN_OP_OUT_REVERSE:
> +	return " out_rev";
> +    case PIXMAN_OP_ATOP:
> +	return "    atop";
> +    case PIXMAN_OP_ATOP_REVERSE:
> +	return "atop_rev";
> +    case PIXMAN_OP_XOR:
> +	return "     xor";
> +    case PIXMAN_OP_ADD:
> +	return "     add";
> +    case PIXMAN_OP_SATURATE:
> +	return "     sat";
> +    default:
> +	return "     etc";
> +    };
> +}

This could be done a little more compactly by storing the names in a
table indexed by the operator.


Soren