Re: [Mesa-dev] [PATCH] [RFC v3] mesa/glthread: Call unmarshal_batch directly in glthread_finish

2017-03-31 Thread Nicolai Hähnle 

On 30.03.2017 14:20, Bartosz Tomczyk wrote:

Thanks Nicolai,

Adding Timothy who seems most active on glthread topic.

Guys, do you think we can land this, with above comments addressed?


Well, Edmondo's testing suggests the performance looks good, so I'd say 
go for it, and we'll see if anything regresses.


Cheers,
Nicolai




On Thu, Mar 30, 2017 at 1:40 PM, Nicolai Hähnle > wrote:

On 30.03.2017 10:30, Bartosz Tomczyk wrote:

Thank you guys for testing.

I'll address all issues in next patch if we decide to merge it.

Nicolai,

Could you comment on _glapi_{get/set}_dispatch part. I'm not
familiar with it and I'm not sure if it's correct.


It looks good to me.

Cheers,
Nicolai


On Thu, Mar 30, 2017 at 5:38 AM, Michel Dänzer

>> wrote:

On 30/03/17 02:31 AM, Bartosz Tomczyk wrote:
> Call it directly when batch queue is empty. This avoids
costly thread
> synchronisation. With this fix games that previously regressed
> with mesa_glthread=true like xonotic or grid autosport.

The second sentence here is missing a verb (at least).


--
Earthling Michel Dänzer   |
 http://www.amd.com
Libre software enthusiast | Mesa and
X developer




--
Lerne, wie die Welt wirklich ist,
Aber vergiss niemals, wie sie sein sollte.





--
Lerne, wie die Welt wirklich ist,
Aber vergiss niemals, wie sie sein sollte.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] [RFC v3] mesa/glthread: Call unmarshal_batch directly in glthread_finish

2017-03-30 Thread Bartosz Tomczyk 
Thanks Nicolai,

Adding Timothy who seems most active on glthread topic.

Guys, do you think we can land this, with above comments addressed?

On Thu, Mar 30, 2017 at 1:40 PM, Nicolai Hähnle  wrote:

> On 30.03.2017 10:30, Bartosz Tomczyk wrote:
>
>> Thank you guys for testing.
>>
>> I'll address all issues in next patch if we decide to merge it.
>>
>> Nicolai,
>>
>> Could you comment on _glapi_{get/set}_dispatch part. I'm not
>> familiar with it and I'm not sure if it's correct.
>>
>
> It looks good to me.
>
> Cheers,
> Nicolai
>
>
>> On Thu, Mar 30, 2017 at 5:38 AM, Michel Dänzer > > wrote:
>>
>> On 30/03/17 02:31 AM, Bartosz Tomczyk wrote:
>> > Call it directly when batch queue is empty. This avoids costly
>> thread
>> > synchronisation. With this fix games that previously regressed
>> > with mesa_glthread=true like xonotic or grid autosport.
>>
>> The second sentence here is missing a verb (at least).
>>
>>
>> --
>> Earthling Michel Dänzer   |
>> http://www.amd.com
>> Libre software enthusiast | Mesa and X
>> developer
>>
>>
>>
>
> --
> Lerne, wie die Welt wirklich ist,
> Aber vergiss niemals, wie sie sein sollte.
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] [RFC v3] mesa/glthread: Call unmarshal_batch directly in glthread_finish

2017-03-30 Thread Nicolai Hähnle 

On 30.03.2017 10:30, Bartosz Tomczyk wrote:

Thank you guys for testing.

I'll address all issues in next patch if we decide to merge it.

Nicolai,

Could you comment on _glapi_{get/set}_dispatch part. I'm not
familiar with it and I'm not sure if it's correct.


It looks good to me.

Cheers,
Nicolai



On Thu, Mar 30, 2017 at 5:38 AM, Michel Dänzer > wrote:

On 30/03/17 02:31 AM, Bartosz Tomczyk wrote:
> Call it directly when batch queue is empty. This avoids costly thread
> synchronisation. With this fix games that previously regressed
> with mesa_glthread=true like xonotic or grid autosport.

The second sentence here is missing a verb (at least).


--
Earthling Michel Dänzer   |   http://www.amd.com
Libre software enthusiast | Mesa and X developer





--
Lerne, wie die Welt wirklich ist,
Aber vergiss niemals, wie sie sein sollte.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] [RFC v3] mesa/glthread: Call unmarshal_batch directly in glthread_finish

2017-03-30 Thread Bartosz Tomczyk 
Thank you guys for testing.

I'll address all issues in next patch if we decide to merge it.

Nicolai,

Could you comment on _glapi_{get/set}_dispatch part. I'm not familiar with
it and I'm not sure if it's correct.

On Thu, Mar 30, 2017 at 5:38 AM, Michel Dänzer  wrote:

> On 30/03/17 02:31 AM, Bartosz Tomczyk wrote:
> > Call it directly when batch queue is empty. This avoids costly thread
> > synchronisation. With this fix games that previously regressed
> > with mesa_glthread=true like xonotic or grid autosport.
>
> The second sentence here is missing a verb (at least).
>
>
> --
> Earthling Michel Dänzer   |   http://www.amd.com
> Libre software enthusiast | Mesa and X developer
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] [RFC v3] mesa/glthread: Call unmarshal_batch directly in glthread_finish

2017-03-29 Thread Michel Dänzer 
On 30/03/17 02:31 AM, Bartosz Tomczyk wrote:
> Call it directly when batch queue is empty. This avoids costly thread
> synchronisation. With this fix games that previously regressed
> with mesa_glthread=true like xonotic or grid autosport.

The second sentence here is missing a verb (at least).


-- 
Earthling Michel Dänzer   |   http://www.amd.com
Libre software enthusiast | Mesa and X developer
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] [RFC v3] mesa/glthread: Call unmarshal_batch directly in glthread_finish

2017-03-29 Thread Thomas Helland 
2017-03-29 21:17 GMT+02:00 Thomas Helland :
> 2017-03-29 19:35 GMT+02:00 Bartosz Tomczyk :
>> I  would be very grateful if someone could help with testing performance
>> impact of this change.
>>
>
> Currently prepping some tests on my HTPC, which is a bit CPU-bound.
> I'll report back in about an hour or so.
>

My HTPC has a RX460, i3-6100 combination, so I was thinking
the low number of threads on the processor could be impacted
by the threaded dispatch. However, I've tested Talos Principle,
Dota 2, and Metro Last Light, and none of these show any
regressions as could be seen in Michael Larabels tests on
phoronix back in mid-March. My system is probably to
GPU-limited for any possible bottleneck to show.

I'll see if I can get my workstation up and running.
It has an RX480, and FX-8320 combination.
So weaker cores, and stronger graphics card.
Hopefully I will be able to reproduce things there.

>> On Wed, Mar 29, 2017 at 7:31 PM, Bartosz Tomczyk
>>  wrote:
>>>
>>> Call it directly when batch queue is empty. This avoids costly thread
>>> synchronisation. With this fix games that previously regressed
>>> with mesa_glthread=true like xonotic or grid autosport.
>>> ---
>>>  src/mesa/main/glthread.c | 47
>>> ++-
>>>  1 file changed, 34 insertions(+), 13 deletions(-)
>>>
>>> diff --git a/src/mesa/main/glthread.c b/src/mesa/main/glthread.c
>>> index 06115b916d..faf42c2b89 100644
>>> --- a/src/mesa/main/glthread.c
>>> +++ b/src/mesa/main/glthread.c
>>> @@ -194,16 +194,12 @@ _mesa_glthread_restore_dispatch(struct gl_context
>>> *ctx)
>>> }
>>>  }
>>>
>>> -void
>>> -_mesa_glthread_flush_batch(struct gl_context *ctx)
>>> +static void
>>> +_mesa_glthread_flush_batch_locked(struct gl_context *ctx)
>>>  {
>>> struct glthread_state *glthread = ctx->GLThread;
>>> -   struct glthread_batch *batch;
>>> -
>>> -   if (!glthread)
>>> -  return;
>>> -
>>> -   batch = glthread->batch;
>>> +   struct glthread_batch *batch = glthread->batch;
>>> +
>>> if (!batch->used)
>>>return;
>>>
>>> @@ -223,10 +219,26 @@ _mesa_glthread_flush_batch(struct gl_context *ctx)
>>>return;
>>> }
>>>
>>> -   pthread_mutex_lock(>mutex);
>>> *glthread->batch_queue_tail = batch;
>>> glthread->batch_queue_tail = >next;
>>> pthread_cond_broadcast(>new_work);
>>> +
>>> +}
>>> +void
>>> +_mesa_glthread_flush_batch(struct gl_context *ctx)
>>> +{
>>> +   struct glthread_state *glthread = ctx->GLThread;
>>> +   struct glthread_batch *batch;
>>> +
>>> +   if (!glthread)
>>> +  return;
>>> +
>>> +   batch = glthread->batch;
>>> +   if (!batch->used)
>>> +  return;
>>> +
>>> +   pthread_mutex_lock(>mutex);
>>> +   _mesa_glthread_flush_batch_locked(ctx);
>>> pthread_mutex_unlock(>mutex);
>>>  }
>>>
>>> @@ -252,12 +264,21 @@ _mesa_glthread_finish(struct gl_context *ctx)
>>> if (pthread_self() == glthread->thread)
>>>return;
>>>
>>> -   _mesa_glthread_flush_batch(ctx);
>>> -
>>> pthread_mutex_lock(>mutex);
>>>
>>> -   while (glthread->batch_queue || glthread->busy)
>>> -  pthread_cond_wait(>work_done, >mutex);
>>> +   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);
>>> + _glapi_set_dispatch(dispatch);
>>> + glthread_allocate_batch(ctx);
>>> +  }
>>> +   }
>>> +   else {
>>> +  _mesa_glthread_flush_batch_locked(ctx);
>>> +  while (glthread->batch_queue || glthread->busy)
>>> + pthread_cond_wait(>work_done, >mutex);
>>> +   }
>>>
>>> pthread_mutex_unlock(>mutex);
>>>  }
>>> --
>>> 2.12.2
>>>
>>
>>
>> ___
>> mesa-dev mailing list
>> mesa-dev@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] [RFC v3] mesa/glthread: Call unmarshal_batch directly in glthread_finish

2017-03-29 Thread Edmondo Tommasina 
This patch helps against the massive performance drop of glthread with
Two Worlds.

The performance boost in Civ5 is not hurt by this patch. It looks good.

Some trivial comments in the patch:

On Wed, Mar 29, 2017 at 7:35 PM, Bartosz Tomczyk
 wrote:
> I  would be very grateful if someone could help with testing performance
> impact of this change.
>
> On Wed, Mar 29, 2017 at 7:31 PM, Bartosz Tomczyk
>  wrote:
>>
>> Call it directly when batch queue is empty. This avoids costly thread
>> synchronisation. With this fix games that previously regressed
>> with mesa_glthread=true like xonotic or grid autosport.
>> ---
>>  src/mesa/main/glthread.c | 47
>> ++-
>>  1 file changed, 34 insertions(+), 13 deletions(-)
>>
>> diff --git a/src/mesa/main/glthread.c b/src/mesa/main/glthread.c
>> index 06115b916d..faf42c2b89 100644
>> --- a/src/mesa/main/glthread.c
>> +++ b/src/mesa/main/glthread.c
>> @@ -194,16 +194,12 @@ _mesa_glthread_restore_dispatch(struct gl_context
>> *ctx)
>> }
>>  }
>>
>> -void
>> -_mesa_glthread_flush_batch(struct gl_context *ctx)
>> +static void
>> +_mesa_glthread_flush_batch_locked(struct gl_context *ctx)
>>  {
>> struct glthread_state *glthread = ctx->GLThread;
>> -   struct glthread_batch *batch;
>> -
>> -   if (!glthread)
>> -  return;
>> -
>> -   batch = glthread->batch;
>> +   struct glthread_batch *batch = glthread->batch;
>> +

Trailing whitespace.

>> if (!batch->used)
>>return;
>>
>> @@ -223,10 +219,26 @@ _mesa_glthread_flush_batch(struct gl_context *ctx)
>>return;
>> }
>>
>> -   pthread_mutex_lock(>mutex);
>> *glthread->batch_queue_tail = batch;
>> glthread->batch_queue_tail = >next;
>> pthread_cond_broadcast(>new_work);
>> +
>> +}

Move the the bracket one line up.

Thanks
edmondo

>> +void
>> +_mesa_glthread_flush_batch(struct gl_context *ctx)
>> +{
>> +   struct glthread_state *glthread = ctx->GLThread;
>> +   struct glthread_batch *batch;
>> +
>> +   if (!glthread)
>> +  return;
>> +
>> +   batch = glthread->batch;
>> +   if (!batch->used)
>> +  return;
>> +
>> +   pthread_mutex_lock(>mutex);
>> +   _mesa_glthread_flush_batch_locked(ctx);
>> pthread_mutex_unlock(>mutex);
>>  }
>>
>> @@ -252,12 +264,21 @@ _mesa_glthread_finish(struct gl_context *ctx)
>> if (pthread_self() == glthread->thread)
>>return;
>>
>> -   _mesa_glthread_flush_batch(ctx);
>> -
>> pthread_mutex_lock(>mutex);
>>
>> -   while (glthread->batch_queue || glthread->busy)
>> -  pthread_cond_wait(>work_done, >mutex);
>> +   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);
>> + _glapi_set_dispatch(dispatch);
>> + glthread_allocate_batch(ctx);
>> +  }
>> +   }
>> +   else {
>> +  _mesa_glthread_flush_batch_locked(ctx);
>> +  while (glthread->batch_queue || glthread->busy)
>> + pthread_cond_wait(>work_done, >mutex);
>> +   }
>>
>> pthread_mutex_unlock(>mutex);
>>  }
>> --
>> 2.12.2
>>
>
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] [RFC v3] mesa/glthread: Call unmarshal_batch directly in glthread_finish

2017-03-29 Thread Thomas Helland 
2017-03-29 19:35 GMT+02:00 Bartosz Tomczyk :
> I  would be very grateful if someone could help with testing performance
> impact of this change.
>

Currently prepping some tests on my HTPC, which is a bit CPU-bound.
I'll report back in about an hour or so.

> On Wed, Mar 29, 2017 at 7:31 PM, Bartosz Tomczyk
>  wrote:
>>
>> Call it directly when batch queue is empty. This avoids costly thread
>> synchronisation. With this fix games that previously regressed
>> with mesa_glthread=true like xonotic or grid autosport.
>> ---
>>  src/mesa/main/glthread.c | 47
>> ++-
>>  1 file changed, 34 insertions(+), 13 deletions(-)
>>
>> diff --git a/src/mesa/main/glthread.c b/src/mesa/main/glthread.c
>> index 06115b916d..faf42c2b89 100644
>> --- a/src/mesa/main/glthread.c
>> +++ b/src/mesa/main/glthread.c
>> @@ -194,16 +194,12 @@ _mesa_glthread_restore_dispatch(struct gl_context
>> *ctx)
>> }
>>  }
>>
>> -void
>> -_mesa_glthread_flush_batch(struct gl_context *ctx)
>> +static void
>> +_mesa_glthread_flush_batch_locked(struct gl_context *ctx)
>>  {
>> struct glthread_state *glthread = ctx->GLThread;
>> -   struct glthread_batch *batch;
>> -
>> -   if (!glthread)
>> -  return;
>> -
>> -   batch = glthread->batch;
>> +   struct glthread_batch *batch = glthread->batch;
>> +
>> if (!batch->used)
>>return;
>>
>> @@ -223,10 +219,26 @@ _mesa_glthread_flush_batch(struct gl_context *ctx)
>>return;
>> }
>>
>> -   pthread_mutex_lock(>mutex);
>> *glthread->batch_queue_tail = batch;
>> glthread->batch_queue_tail = >next;
>> pthread_cond_broadcast(>new_work);
>> +
>> +}
>> +void
>> +_mesa_glthread_flush_batch(struct gl_context *ctx)
>> +{
>> +   struct glthread_state *glthread = ctx->GLThread;
>> +   struct glthread_batch *batch;
>> +
>> +   if (!glthread)
>> +  return;
>> +
>> +   batch = glthread->batch;
>> +   if (!batch->used)
>> +  return;
>> +
>> +   pthread_mutex_lock(>mutex);
>> +   _mesa_glthread_flush_batch_locked(ctx);
>> pthread_mutex_unlock(>mutex);
>>  }
>>
>> @@ -252,12 +264,21 @@ _mesa_glthread_finish(struct gl_context *ctx)
>> if (pthread_self() == glthread->thread)
>>return;
>>
>> -   _mesa_glthread_flush_batch(ctx);
>> -
>> pthread_mutex_lock(>mutex);
>>
>> -   while (glthread->batch_queue || glthread->busy)
>> -  pthread_cond_wait(>work_done, >mutex);
>> +   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);
>> + _glapi_set_dispatch(dispatch);
>> + glthread_allocate_batch(ctx);
>> +  }
>> +   }
>> +   else {
>> +  _mesa_glthread_flush_batch_locked(ctx);
>> +  while (glthread->batch_queue || glthread->busy)
>> + pthread_cond_wait(>work_done, >mutex);
>> +   }
>>
>> pthread_mutex_unlock(>mutex);
>>  }
>> --
>> 2.12.2
>>
>
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] [RFC v3] mesa/glthread: Call unmarshal_batch directly in glthread_finish

2017-03-29 Thread Bartosz Tomczyk 
I  would be very grateful if someone could help with testing performance
impact of this change.

On Wed, Mar 29, 2017 at 7:31 PM, Bartosz Tomczyk <
bartosz.tomczy...@gmail.com> wrote:

> Call it directly when batch queue is empty. This avoids costly thread
> synchronisation. With this fix games that previously regressed
> with mesa_glthread=true like xonotic or grid autosport.
> ---
>  src/mesa/main/glthread.c | 47 ++
> -
>  1 file changed, 34 insertions(+), 13 deletions(-)
>
> diff --git a/src/mesa/main/glthread.c b/src/mesa/main/glthread.c
> index 06115b916d..faf42c2b89 100644
> --- a/src/mesa/main/glthread.c
> +++ b/src/mesa/main/glthread.c
> @@ -194,16 +194,12 @@ _mesa_glthread_restore_dispatch(struct gl_context
> *ctx)
> }
>  }
>
> -void
> -_mesa_glthread_flush_batch(struct gl_context *ctx)
> +static void
> +_mesa_glthread_flush_batch_locked(struct gl_context *ctx)
>  {
> struct glthread_state *glthread = ctx->GLThread;
> -   struct glthread_batch *batch;
> -
> -   if (!glthread)
> -  return;
> -
> -   batch = glthread->batch;
> +   struct glthread_batch *batch = glthread->batch;
> +
> if (!batch->used)
>return;
>
> @@ -223,10 +219,26 @@ _mesa_glthread_flush_batch(struct gl_context *ctx)
>return;
> }
>
> -   pthread_mutex_lock(>mutex);
> *glthread->batch_queue_tail = batch;
> glthread->batch_queue_tail = >next;
> pthread_cond_broadcast(>new_work);
> +
> +}
> +void
> +_mesa_glthread_flush_batch(struct gl_context *ctx)
> +{
> +   struct glthread_state *glthread = ctx->GLThread;
> +   struct glthread_batch *batch;
> +
> +   if (!glthread)
> +  return;
> +
> +   batch = glthread->batch;
> +   if (!batch->used)
> +  return;
> +
> +   pthread_mutex_lock(>mutex);
> +   _mesa_glthread_flush_batch_locked(ctx);
> pthread_mutex_unlock(>mutex);
>  }
>
> @@ -252,12 +264,21 @@ _mesa_glthread_finish(struct gl_context *ctx)
> if (pthread_self() == glthread->thread)
>return;
>
> -   _mesa_glthread_flush_batch(ctx);
> -
> pthread_mutex_lock(>mutex);
>
> -   while (glthread->batch_queue || glthread->busy)
> -  pthread_cond_wait(>work_done, >mutex);
> +   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);
> + _glapi_set_dispatch(dispatch);
> + glthread_allocate_batch(ctx);
> +  }
> +   }
> +   else {
> +  _mesa_glthread_flush_batch_locked(ctx);
> +  while (glthread->batch_queue || glthread->busy)
> + pthread_cond_wait(>work_done, >mutex);
> +   }
>
> pthread_mutex_unlock(>mutex);
>  }
> --
> 2.12.2
>
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev