[Mesa-dev] [RFC] Gallium constant handling speed improvements

[Brian Paul]

On 12/03/2010 12:47 PM, Jakob Bornecrantz wrote:
> Hi all
>
> I have attached two patches that should help with speed on applications that
> causes a lot of constant buffers to be emitted or lot of constant state as in
> the case of the second patch as it eliminates a memcpy.
>
> The first patch just adds a offset and size argument to the set_constant_buffer
> call on the context, making it possible for the state tracker sub-allocate the
> buffers out a larger buffer. For ipers this is a pretty big win. Even if we did
> something similar inside the drivers various locks would have to be taken
> because since create buffer is a screen function.
>
> The second patch switches from a inline write to map buffer call and points the
> _mesa_load_state_parameters to it directly. For ipers this is a minor loss but
> for application that has a different ratio of number of constants vs number of
> times the constants changes it will probably be a win.

I don't see how this can work as-is.  We need to put all the 
parameters/constants into the buffer at the new offset, not just the 
PROGRAM_STATE_VARs.  Recall the the Mesa parameter/constant buffer 
constains PROGRAM_STATE_VAR (i.e. fixed-function state) plus the 
user's uniforms, program parameters, shader literals, etc.

You should test some GLSL programs too.

In _mesa_load_state_parameters_to() you might try adding an else 
clause to the Type == PROGRAM_STATE_VAR conditional which copies the 
current parameter value to the dest buffer.


> I have tested the patches on i915g only, unfortunately I based the i915 part
> of these patches on some other work I had laying around so there is no point
> in sending them out since they wont apply.
>
> Before I go and try and change the other drivers I would like to hear some input
> on these changes. So comment please, anyways have a nice weekend.


[...]

>
> diff --git a/src/mesa/state_tracker/st_context.c b/src/mesa/state_tracker/st_context.c
> index d6628b1..303b924 100644
> --- a/src/mesa/state_tracker/st_context.c
> +++ b/src/mesa/state_tracker/st_context.c
> @@ -244,8 +244,8 @@ void st_destroy_context( struct st_context *st )
>     pipe->set_index_buffer(pipe, NULL);
>
>     for (i = 0; i < PIPE_SHADER_TYPES; i++) {
> -      pipe->set_constant_buffer(pipe, PIPE_SHADER_VERTEX, 0, NULL);
> -      pipe_resource_reference(&st->state.constants[PIPE_SHADER_VERTEX], NULL);
> +      pipe->set_constant_buffer(pipe, i, 0, 0, 0, NULL);
> +      pipe_resource_reference(&st->state.constants[i], NULL);
>     }

This bug fix should be committed soon in any case.  Looks like it 
comes from Tilman's commit 646a8b7e1dfbd126819d577a94102e64748d0b9d


-Brian