[Mesa-dev] [PATCH 19/87] glsl: cache more uniform storage values

[Timothy Arceri]

On Thu, 2016-07-28 at 13:35 +0300, Tapani Pälli wrote:
> 
> On 07/13/2016 05:47 AM, Timothy Arceri wrote:
> > ---
> >  src/compiler/glsl/shader_cache.cpp | 16 ++++++++++++++--
> >  1 file changed, 14 insertions(+), 2 deletions(-)
> > 
> > diff --git a/src/compiler/glsl/shader_cache.cpp
> > b/src/compiler/glsl/shader_cache.cpp
> > index 08f8e37..8aae1c8 100644
> > --- a/src/compiler/glsl/shader_cache.cpp
> > +++ b/src/compiler/glsl/shader_cache.cpp
> > @@ -136,10 +136,17 @@ write_uniforms(struct blob *metadata, struct
> > gl_shader_program *prog)
> > 
> >     for (i = 0; i < prog->NumUniformStorage; i++) {
> >        encode_type_to_blob(metadata, prog->UniformStorage[i].type);
> > +      blob_write_uint32(metadata, prog-
> > >UniformStorage[i].array_elements);
> >        blob_write_string(metadata, prog->UniformStorage[i].name);
> >        blob_write_uint32(metadata, prog->UniformStorage[i].storage
> > -
> >                                    prog->UniformDataSlots);
> >        blob_write_uint32(metadata, prog-
> > >UniformStorage[i].remap_location);
> > +      blob_write_uint32(metadata, prog-
> > >UniformStorage[i].block_index);
> > +      blob_write_uint32(metadata, prog-
> > >UniformStorage[i].atomic_buffer_index);
> > +      blob_write_uint32(metadata, prog->UniformStorage[i].offset);
> > +      blob_write_uint32(metadata, prog-
> > >UniformStorage[i].array_stride);
> > +      blob_write_uint32(metadata, prog-
> > >UniformStorage[i].matrix_stride);
> > +      blob_write_uint32(metadata, prog-
> > >UniformStorage[i].row_major);
> 
> A proposal for this .. IMO it would make sense to write the whole 
> structure + few pointer fields instead of writing/reading all these 
> individual fields separately. We could change them to sized types 
> (uint32_t ..) in the structure. I found this brought a lot of more
> speed 
> to IR cache I worked on previously. So basically you could just read
> and 
> write gl_uniform_storage as a blob it is and then read/fill 'name', 
> 'type' and other fields behind pointer, this will be a lot less 
> reads/writes. Same goes with other data structures like gl_shader.
> 
> (I also saved sizes of all serialized structures so that if a
> structure 
> in driver code gets changed, cache entry gets deleted and replaced
> with 
> a new one.)

Thanks for taking a look over this. My hope was to be able to land
something that works then focus on performance improvements later. I
have not done any profiling on the current code and your suggestion
would likely be a good place to start improving performance. However
I'd really like to land this series before editing structures which
will probably result in more changes to the linker, although thinking
about it some more we might not need to do this as we don't currently
use the same cached shaders when running 32bit vs 64bit versions of and
applicaion see [1], although this could maybe be improved later too. 
Anyway, I'm seeing good results from the current code with load/store
times seeming more than acceptable at this stage with games running
noticeably smoother.

As for invalidating the cache this series currently uses the Mesa
version string to invalidate the cache. It's possible that is too
aggressive but it should certainly be safe (although I may have to
check what the final solution was for the Vulkan driver as there was an
issue with tarballs).

[1] https://github.com/tarceri/Mesa_arrays_of_arrays/commit/adceb20b2e9
5e9109abd041f89779d3229c4a131

Thanks again for looking over this.

Tim

> 
> 
> >     }
> >  }
> > 
> > @@ -166,11 +173,16 @@ read_uniforms(struct blob_reader *metadata,
> > struct gl_shader_program *prog)
> > 
> >     for (i = 0; i < prog->NumUniformStorage; i++) {
> >        uniforms[i].type = decode_type_from_blob(metadata);
> > +      uniforms[i].array_elements = blob_read_uint32(metadata);
> >        uniforms[i].name = ralloc_strdup(prog, blob_read_string
> > (metadata));
> >        uniforms[i].storage = data + blob_read_uint32(metadata);
> >        uniforms[i].remap_location = blob_read_uint32(metadata);
> > -      uniforms[i].block_index = -1;
> > -      uniforms[i].atomic_buffer_index = -1;
> > +      uniforms[i].block_index = blob_read_uint32(metadata);
> > +      uniforms[i].atomic_buffer_index =
> > blob_read_uint32(metadata);
> > +      uniforms[i].offset = blob_read_uint32(metadata);
> > +      uniforms[i].array_stride = blob_read_uint32(metadata);
> > +      uniforms[i].matrix_stride = blob_read_uint32(metadata);
> > +      uniforms[i].row_major = blob_read_uint32(metadata);
> >        prog->UniformHash->put(i, uniforms[i].name);
> >     }
> >  }
> > 
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev