[Mesa-dev] [PATCH 1/5] u_upload_mgr: optimize u_upload_alloc

[Brian Paul]

On 09/02/2015 07:44 AM, Marek Olšák wrote:
> From: Marek Olšák <marek.olsak at amd.com>
>
> This is probably the most called util function. It does almost nothing,
> yet it can consume 10% of the CPU on the profile. This drops it down to 5%.
> ---
>   src/gallium/auxiliary/util/u_upload_mgr.c | 36 +++++++++++++++----------------
>   1 file changed, 18 insertions(+), 18 deletions(-)
>
> diff --git a/src/gallium/auxiliary/util/u_upload_mgr.c b/src/gallium/auxiliary/util/u_upload_mgr.c
> index 744ea2e..c30386e 100644
> --- a/src/gallium/auxiliary/util/u_upload_mgr.c
> +++ b/src/gallium/auxiliary/util/u_upload_mgr.c
> @@ -186,50 +186,50 @@ enum pipe_error u_upload_alloc( struct u_upload_mgr *upload,
>                                   struct pipe_resource **outbuf,
>                                   void **ptr )
>   {
> -   unsigned alloc_size = align( size, upload->alignment );
> +   unsigned alloc_size = align(size, upload->alignment);
>      unsigned alloc_offset = align(min_out_offset, upload->alignment);
> +   unsigned buffer_size = upload->buffer ? upload->buffer->width0 : 0;
>      unsigned offset;
>
> -   /* Init these return values here in case we fail below to make
> -    * sure the caller doesn't get garbage values.
> -    */
> -   *out_offset = ~0;
> -   pipe_resource_reference(outbuf, NULL);
> -   *ptr = NULL;
> -
>      /* Make sure we have enough space in the upload buffer
>       * for the sub-allocation. */
> -   if (!upload->buffer ||
> -       MAX2(upload->offset, alloc_offset) + alloc_size > upload->buffer->width0) {
> +   if (unlikely(MAX2(upload->offset, alloc_offset) + alloc_size > buffer_size)) {
>         enum pipe_error ret = u_upload_alloc_buffer(upload,
>                                                     alloc_offset + alloc_size);
> -      if (ret != PIPE_OK)
> +      if (unlikely(ret != PIPE_OK)) {
> +         *out_offset = ~0;
> +         pipe_resource_reference(outbuf, NULL);
> +         *ptr = NULL;
>            return ret;
> +      }
> +
> +      buffer_size = upload->buffer->width0;
>      }
>
>      offset = MAX2(upload->offset, alloc_offset);
>
> -   if (!upload->map) {
> +   if (unlikely(!upload->map)) {
>         upload->map = pipe_buffer_map_range(upload->pipe, upload->buffer,
>                                             offset,
> -                                          upload->buffer->width0 - offset,
> +                                          buffer_size - offset,
>                                             upload->map_flags,
>   					  &upload->transfer);
> -      if (!upload->map) {
> +      if (unlikely(!upload->map)) {
>            upload->transfer = NULL;
> +         *out_offset = ~0;
> +         pipe_resource_reference(outbuf, NULL);
> +         *ptr = NULL;
>            return PIPE_ERROR_OUT_OF_MEMORY;
>         }
>
>         upload->map -= offset;
>      }
>
> -   assert(offset < upload->buffer->width0);
> -   assert(offset + size <= upload->buffer->width0);
> -   assert(size);
> +   assert(offset < buffer_size && offset + size <= buffer_size && size);

I'd prefer three separate assertions.  If the assertion fails as-is, you 
can't know which part failed.

>
>      /* Emit the return values: */
>      *ptr = upload->map + offset;
> -   pipe_resource_reference( outbuf, upload->buffer );
> +   pipe_resource_reference(outbuf, upload->buffer);
>      *out_offset = offset;
>
>      upload->offset = offset + alloc_size;
>

Reviewed-by: Brian Paul <brianp at vmware.com>