[Mesa-dev] [PATCH 0/7] vbo: introduce a minmax_index cache

[Nicolai Hähnle]

On 08.01.2016 18:11, Ian Romanick wrote:
> On 01/07/2016 04:57 PM, Nicolai Hähnle wrote:
>> Hi,
>>
>> this series is intended to address a (big) part of the performance problems
>> that occur when games use an index buffer from a VBO together with vertex
>> attributes supplied by user pointer. On a lower-end Radeon (Carrizo), it
>
> This is one of (several) reasons that I've taken such a hard stance
> against compatibility profile over the years. :(  In core profile, you
> can't have vertex attributes in user memory.
>
>> lifts This War Of Mine (the worst offender I've seen) from basically unplayable
>> to an acceptable ~22fps.
>
> That's a really good result!  Have you tried this on low-CPU systems
> with applications that aren't as pessimal?

No. Do you have particular applications in mind? The typical benchmarks 
don't fall into this trap, and so this series only affects them via the 
additional UsageHistory and the MinMaxCacheDirty flags.

> There are a couple things I was hoping to see in this series that aren't
> here.
>
> 1. Collection of hit-rate data.

Fair point.

> 2. A way to disable the cache per-application.  Since caching can
> already be disable per-BO, this would be trivial to add via another
> USAGE_ flag.

Are you thinking of a drirc setting or a MESA_* environment variable?

> I can imagine a few scenarios where the simple caching system used here
> could cause an application to perform worse.  One such case isn't even
> that crazy:
>
> 1. Fill the BO with new element data (which invalidates the cache).
>
> 2. Stream through the BO performing a bunch of small draw calls from a
> different range of data.  Each draw will miss the cache and will add a
> new entry.
>
> 3. Goto 1.

How about this for a start: Keep a count of cache hits per buffer 
object. When the cache is invalidated, compare to the number of entries 
in the cache. If it's below a certain percentage threshold, set a 
UsageHistory flag disabling the cache for this BO.

I know that there are also applications that create BOs at runtime 
(dynamic model loading as far as I can tell), but I doubt anybody is 
silly enough to create one-shot BOs, so we'd learn relatively quickly 
how each BO is used.

I'm going to try this out next week.

Thanks for the review!
Nicolai

> I'd really want to have data to know that the cache is hurting
> performance.  With that data, I'd also want to disable the cache in that
> application while I find a way to either improve the cache utilization
> or detect the usage pattern to automatically disable the cache.
>
> That said, patches 1, 2, 3, 5, and 6 are
>
> Reviewed-by: Ian Romanick <ian.d.romanick at intel.com>
>
> I sent a comment on 4 and a small nit on 7.
>
>> There are really two parts to the performance inefficiency:
>> (1) the fact that we have to compute min/max in the first place
>> (2) the fact that games developers like to helpfully declare the index buffer
>> as STATIC_DRAW, which means it ends up in VRAM, making things much worse than
>> they would be with (1) alone.
>>
>> The series does this simply by caching (or memoizing, if you like) the result
>> of vbo_get_minmax_indices. This makes sense since while the caching itself of
>> course has some overhead, it is only used for VBOs - and the people who mix
>> VBO index buffers with non-VBO vertex data seem to do so precisely because
>> their index buffers are static, hence caching is a win.
>>
>> The cache is disabled permanently for VBOs that we detect to be written to by
>> GPU operations. It is invalidated on other writes (Buffer(Sub)Data etc.).
>>
>> I originally considered putting the caching in st/mesa instead of vbo, but that
>> would have required duplicating some of the logic in vbo_get_minmax_indices for
>> iterating over primitives inside the Gallium statetracker. I wanted to avoid
>> this code duplication, hence why I put it in vbo.
>>
>> Please review!
>>
>> Thanks,
>> Nicolai
>> --
>>   src/mesa/Makefile.sources             |   1 +
>>   src/mesa/main/bufferobj.c             |  41 +++-
>>   src/mesa/main/mtypes.h                |   7 +
>>   src/mesa/main/transformfeedback.h     |   3 +
>>   src/mesa/vbo/vbo.h                    |   3 +
>>   src/mesa/vbo/vbo_exec_array.c         | 148 ------------
>>   src/mesa/vbo/vbo_minmax_index.c       | 342 ++++++++++++++++++++++++++++
>>   src/util/hash_table.c                 |  25 ++
>>   src/util/hash_table.h                 |   2 +
>>   src/util/tests/hash_table/Makefile.am |   1 +
>>   src/util/tests/hash_table/clear.c     |  91 ++++++++
>>   11 files changed, 507 insertions(+), 157 deletions(-)
>>
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>>
>