[Mesa-dev] [PATCH] mesa: Free the compiled shader IR after it has been linked.

[Ian Romanick]

On 05/28/2014 01:57 PM, Eric Anholt wrote:
> If the shader compiled once, then we can compile it again.  Compiled
> shaders almost always get used in just one program, so holding that
> compiled IR until the program is freed is just a waste of memory.

Would this work with some madness like:

   glAttachShader(prog, sh1);
   glAttachShader(prog, sh2);
   glLinkProgram(prog);

   GLchar *empty = "";
   glShaderSource(sh1, 1, &empty, NULL);

   glBindAttribLocation(prog, 0, "foo");
   glLinkProgram(prog);

If I'm reading this patch correctly, it will try to recompile the new
souce string on the second glLinkProgram.

There are some things like this that I think would work very nicely with
glCreateShaderProgramv, but it may be difficult to get all the corner
cases right otherwise.

> On the other hand, if they are either reusing shader objects to compile
> multiple times, or linking the same shader into multiple programs, we turn
> off this memory savings hack to avoid spending CPU on recompiling.
> 
> Reduces peak memory allocation of glretrace of a trace of dolphin-emu by
> 5.5MB.  It seems like this should be a big deal to DOTA2, but it was
> triggering RecompiledAnyShader, and I failed to see a benefit even if I
> removed the RecompiledAnyShader check (which confuses me).

I think they use the same vertex shader with multiple fragment
shaders... but it does seem weird that disabling the check didn't change
the memory usage.  Did it change anything (bad rendering)?

> ---
>  src/mesa/main/mtypes.h          |  6 ++++++
>  src/mesa/main/shaderapi.c       | 19 +++++++++++++++++++
>  src/mesa/main/shaderapi.h       |  3 +++
>  src/mesa/program/ir_to_mesa.cpp | 23 ++++++++++++++++++++++-
>  4 files changed, 50 insertions(+), 1 deletion(-)
> 
> diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h
> index 917d071..79a1514 100644
> --- a/src/mesa/main/mtypes.h
> +++ b/src/mesa/main/mtypes.h
> @@ -4210,6 +4210,12 @@ struct gl_context
>     /*@}*/
>  
>     /**
> +    * Set if we've had to recompile any shaders in order to link them into a
> +    * new program after we'd freed their IR.
> +    */
> +   bool RecompiledAnyShaders;
> +
> +   /**
>      * False if this context was created without a config. This is needed
>      * because the initial state of glDrawBuffers depends on this
>      */
> diff --git a/src/mesa/main/shaderapi.c b/src/mesa/main/shaderapi.c
> index 28739da..44e8db6 100644
> --- a/src/mesa/main/shaderapi.c
> +++ b/src/mesa/main/shaderapi.c
> @@ -803,6 +803,8 @@ shader_source(struct gl_context *ctx, GLuint shader, const GLchar *source)
>     if (!sh)
>        return;
>  
> +   _mesa_glsl_lazy_recompile_ir(ctx, sh);
> +
>     /* free old shader source string and install new one */
>     free((void *)sh->Source);
>     sh->Source = source;
> @@ -812,6 +814,23 @@ shader_source(struct gl_context *ctx, GLuint shader, const GLchar *source)
>  #endif
>  }
>  
> +/**
> + * Used to ensure that there is a valid set of IR in the program when we might
> + * have freed it after glLinkProgram() on the assumption that nobody would go
> + * looking for it again.
> +*/
> +void
> +_mesa_glsl_lazy_recompile_ir(struct gl_context *ctx, struct gl_shader *sh)
> +{
> +   if (sh->ir || !sh->CompileStatus)
> +      return;
> +
> +   _mesa_glsl_compile_shader(ctx, sh, false, false);
> +   /* The recompile had better have succeeded! */
> +   assert(sh->CompileStatus);
> +
> +   ctx->RecompiledAnyShaders = true;
> +}
>  
>  /**
>   * Compile a shader.
> diff --git a/src/mesa/main/shaderapi.h b/src/mesa/main/shaderapi.h
> index 17b05b3..4e6c31f 100644
> --- a/src/mesa/main/shaderapi.h
> +++ b/src/mesa/main/shaderapi.h
> @@ -228,6 +228,9 @@ extern GLuint GLAPIENTRY
>  _mesa_CreateShaderProgramv(GLenum type, GLsizei count,
>                             const GLchar* const *strings);
>  
> +void
> +_mesa_glsl_lazy_recompile_ir(struct gl_context *ctx, struct gl_shader *sh);
> +
>  #ifdef __cplusplus
>  }
>  #endif
> diff --git a/src/mesa/program/ir_to_mesa.cpp b/src/mesa/program/ir_to_mesa.cpp
> index 59cf123..d4b66bf 100644
> --- a/src/mesa/program/ir_to_mesa.cpp
> +++ b/src/mesa/program/ir_to_mesa.cpp
> @@ -3079,8 +3079,14 @@ _mesa_glsl_link_shader(struct gl_context *ctx, struct gl_shader_program *prog)
>     prog->LinkStatus = GL_TRUE;
>  
>     for (i = 0; i < prog->NumShaders; i++) {
> -      if (!prog->Shaders[i]->CompileStatus) {
> +      struct gl_shader *sh = prog->Shaders[i];
> +      if (!sh->CompileStatus) {
>  	 linker_error(prog, "linking with uncompiled shader");
> +      } else {
> +         /* If we'd freed the compiled IR after a previous link in order to
> +          * save memory, recreate it.
> +          */
> +         _mesa_glsl_lazy_recompile_ir(ctx, sh);
>        }
>     }
>  
> @@ -3104,6 +3110,21 @@ _mesa_glsl_link_shader(struct gl_context *ctx, struct gl_shader_program *prog)
>  	 fprintf(stderr, "%s\n", prog->InfoLog);
>        }
>     }
> +
> +   /* Free the compiled shaders' IR now that they've been linked into a
> +    * program.  This saves memory in the common case of people using each
> +    * shader in one program, at the cost of needing to lazily recompile one
> +    * set of them if somebody does actually use the same gl_shader in multiple
> +    * gl_shader_programs.
> +    */
> +   if (prog->LinkStatus && !ctx->RecompiledAnyShaders) {
> +      for (i = 0; i < prog->NumShaders; i++) {
> +         struct gl_shader *sh = prog->Shaders[i];
> +
> +         ralloc_free(sh->ir);
> +         sh->ir = NULL;
> +      }
> +   }
>  }
>  
>  } /* extern "C" */