[Mesa-dev] [PATCH] mesa/glthread: Avoid unnecessary batch reallocation

[Bartosz Tomczyk]

Actually, I can just set only batch->used to 0, but it seems to error
prone. When someone adds some fields to batch struct, it will be easy to
miss that it should be initialized in glthread_unmarshal_batch.

Anyway I can change it if you want to.

On Mon, Apr 3, 2017 at 8:42 PM, Nicolai Hähnle <nhaehnle at gmail.com> wrote:

> On 03.04.2017 20:38, Bartosz Tomczyk wrote:
>
>> ---
>>  src/mesa/main/glthread.c | 15 +++++++++------
>>  1 file changed, 9 insertions(+), 6 deletions(-)
>>
>> diff --git a/src/mesa/main/glthread.c b/src/mesa/main/glthread.c
>> index 3f07c420d4..aa14292e59 100644
>> --- a/src/mesa/main/glthread.c
>> +++ b/src/mesa/main/glthread.c
>> @@ -53,7 +53,8 @@ glthread_allocate_batch(struct gl_context *ctx)
>>  }
>>
>>  static void
>> -glthread_unmarshal_batch(struct gl_context *ctx, struct glthread_batch
>> *batch)
>> +glthread_unmarshal_batch(struct gl_context *ctx, struct glthread_batch
>> *batch,
>> +                         const bool release_batch)
>>  {
>>     size_t pos = 0;
>>
>> @@ -64,7 +65,10 @@ glthread_unmarshal_batch(struct gl_context *ctx,
>> struct glthread_batch *batch)
>>
>>     assert(pos == batch->used);
>>
>> -   free(batch);
>> +   if (release_batch)
>> +      free(batch);
>> +   else
>> +      memset(batch, 0, offsetof(struct glthread_batch, buffer));
>>
>
> Hmm. Why do we memset the batch? Seems like a trivial optimization to just
> not do that...
>
> Apart from that, the patch looks good to me.
>
> Cheers,
> Nicolai
>
>
>
>  }
>>
>>  static void *
>> @@ -103,7 +107,7 @@ glthread_worker(void *data)
>>        glthread->busy = true;
>>        pthread_mutex_unlock(&glthread->mutex);
>>
>> -      glthread_unmarshal_batch(ctx, batch);
>> +      glthread_unmarshal_batch(ctx, batch, true);
>>
>>        pthread_mutex_lock(&glthread->mutex);
>>        glthread->busy = false;
>> @@ -214,7 +218,7 @@ _mesa_glthread_flush_batch_locked(struct gl_context
>> *ctx)
>>      * need to restore it when it returns.
>>      */
>>     if (false) {
>> -      glthread_unmarshal_batch(ctx, batch);
>> +      glthread_unmarshal_batch(ctx, batch, true);
>>        _glapi_set_dispatch(ctx->CurrentClientDispatch);
>>        return;
>>     }
>> @@ -269,9 +273,8 @@ _mesa_glthread_finish(struct gl_context *ctx)
>>     if (!(glthread->batch_queue || glthread->busy)) {
>>        if (glthread->batch && glthread->batch->used) {
>>           struct _glapi_table *dispatch = _glapi_get_dispatch();
>> -         glthread_unmarshal_batch(ctx, glthread->batch);
>> +         glthread_unmarshal_batch(ctx, glthread->batch, false);
>>           _glapi_set_dispatch(dispatch);
>> -         glthread_allocate_batch(ctx);
>>        }
>>     } else {
>>        _mesa_glthread_flush_batch_locked(ctx);
>>
>>
>
> --
> Lerne, wie die Welt wirklich ist,
> Aber vergiss niemals, wie sie sein sollte.
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20170403/05c5aaf6/attachment.html>