[Mesa-dev] [PATCH] glcpp: Avoid unnecessary call to strlen

Timothy Arceri tarceri at itsqueeze.com
Thu Aug 31 00:20:32 UTC 2017



On 31/08/17 06:19, Thomas Helland wrote:
> Length of the token was already calculated by flex and stored in yyleng,
> no need to implicitly call strlen() via linear_strdup().
> 
> V2: Also convert this pattern in glsl_lexer.ll
>      This has a surprising result of reducing executed cyles from
>      1.06 trillion to 1.03 trillion, a significant change.
>      Perf reports an increase in instructions per cycle from
>      1,04 as a normal value, to 1,07. I'm also seeing a decent
>      reduction in shader-db runtimes, from 113 seconds on master,
>      down to 98 seconds. I'll have to do more thorough testing
>      to verify if this is actually statistically significant,
>      but the change in execution was drastic when adding the change
>      to glsl_lexer.ll, and with the tests run as of now it looks good.

Thanks for the updated patch. Unfortunately I didn't see the same 
improvements in shader-db:

master:

$ time ./run -j 1 shaders > new-run 2> /dev/null

real	9m22.023s
user	9m15.424s
sys	0m3.487s

master with this patch:

$ time ./run -j 1 shaders > new-run 2> /dev/null

real	9m21.611s
user	9m15.682s
sys	0m3.339s

> ---
>   src/compiler/glsl/glcpp/glcpp-lex.l | 13 ++++++++++++-
>   src/compiler/glsl/glsl_lexer.ll     | 32 ++++++++++++++++++++++++++------
>   src/compiler/glsl/glsl_parser.yy    |  2 +-
>   3 files changed, 39 insertions(+), 8 deletions(-)
> 
> diff --git a/src/compiler/glsl/glcpp/glcpp-lex.l b/src/compiler/glsl/glcpp/glcpp-lex.l
> index 381b97364a..a1fbcaa928 100644
> --- a/src/compiler/glsl/glcpp/glcpp-lex.l
> +++ b/src/compiler/glsl/glcpp/glcpp-lex.l
> @@ -98,10 +98,21 @@ void glcpp_set_column (int  column_no , yyscan_t yyscanner);
>   		}							\
>   	} while(0)
>   
> +/* We're not doing linear_strdup here, to avoid an implicit call on
> + * strlen() for the length of the string, as this is already found by
> + * flex and stored in yyleng
> + */
>   #define RETURN_STRING_TOKEN(token)					\
>   	do {								\
>   		if (! parser->skipping) {				\
> -			yylval->str = linear_strdup(yyextra->linalloc, yytext);	\
> +			/* We're not doing linear_strdup here, to avoid \
> +			 * an implicit call on strlen() for the length  \
> +			 * of the string, as this is already found by   \
> +			 * flex and stored in yyleng */                 \
> +			void *mem_ctx = yyextra->linalloc;		\
> +			yylval->str = linear_alloc_child(mem_ctx,	\
> +							 yyleng + 1);	\
> +			memcpy(yylval->str, yytext, yyleng + 1);        \
>   			RETURN_TOKEN_NEVER_SKIP (token);		\
>   		}							\
>   	} while(0)
> diff --git a/src/compiler/glsl/glsl_lexer.ll b/src/compiler/glsl/glsl_lexer.ll
> index d4d7ada1b1..5eceb546b9 100644
> --- a/src/compiler/glsl/glsl_lexer.ll
> +++ b/src/compiler/glsl/glsl_lexer.ll
> @@ -80,8 +80,13 @@ static int classify_identifier(struct _mesa_glsl_parse_state *, const char *);
>   			  "illegal use of reserved word `%s'", yytext);	\
>   	 return ERROR_TOK;						\
>         } else {								\
> -	 void *mem_ctx = yyextra->linalloc;					\
> -	 yylval->identifier = linear_strdup(mem_ctx, yytext);		\
> +	 /* We're not doing linear_strdup here, to avoid an implicit    \
> +	  * call on strlen() for the length of the string, as this is   \
> +	  * already found by flex and stored in yyleng */               \
> +	 void *mem_ctx = yyextra->linalloc;				\
> +	 yylval->identifier = (char *) linear_alloc_child(mem_ctx,	\
> +							  yyleng + 1);	\
> +	 memcpy(yylval->identifier, yytext, yyleng + 1);		\
>   	 return classify_identifier(yyextra, yytext);			\
>         }									\
>      } while (0)
> @@ -260,8 +265,13 @@ HASH		^{SPC}#{SPC}
>   <PP>[ \t\r]*			{ }
>   <PP>:				return COLON;
>   <PP>[_a-zA-Z][_a-zA-Z0-9]*	{
> +				   /* We're not doing linear_strdup here, to avoid an implicit call
> +				    * on strlen() for the length of the string, as this is already
> +				    * found by flex and stored in yyleng
> +				    */
>   				   void *mem_ctx = yyextra->linalloc;
> -				   yylval->identifier = linear_strdup(mem_ctx, yytext);
> +				   yylval->identifier = (char *) linear_alloc_child(mem_ctx, yyleng + 1);
> +				   memcpy(yylval->identifier, yytext, yyleng + 1);
>   				   return IDENTIFIER;
>   				}
>   <PP>[1-9][0-9]*			{
> @@ -448,8 +458,13 @@ layout		{
>                         || yyextra->ARB_tessellation_shader_enable) {
>   		      return LAYOUT_TOK;
>   		   } else {
> +		      /* We're not doing linear_strdup here, to avoid an implicit call
> +		       * on strlen() for the length of the string, as this is already
> +		       * found by flex and stored in yyleng
> +		       */
>   		      void *mem_ctx = yyextra->linalloc;
> -		      yylval->identifier = linear_strdup(mem_ctx, yytext);
> +		      yylval->identifier = (char *) linear_alloc_child(mem_ctx, yyleng + 1);
> +		      memcpy(yylval->identifier, yytext, yyleng + 1);
>   		      return classify_identifier(yyextra, yytext);
>   		   }
>   		}
> @@ -620,12 +635,17 @@ u64vec4		KEYWORD_WITH_ALT(0, 0, 0, 0, yyextra->ARB_gpu_shader_int64_enable, U64V
>   [_a-zA-Z][_a-zA-Z0-9]*	{
>   			    struct _mesa_glsl_parse_state *state = yyextra;
>   			    void *ctx = state->linalloc;
> -			    if (state->es_shader && strlen(yytext) > 1024) {
> +			    if (state->es_shader && yyleng + 1 > 1024) {
>   			       _mesa_glsl_error(yylloc, state,
>   			                        "Identifier `%s' exceeds 1024 characters",
>   			                        yytext);
>   			    } else {
> -			      yylval->identifier = linear_strdup(ctx, yytext);
> +			      /* We're not doing linear_strdup here, to avoid an implicit call
> +			       * on strlen() for the length of the string, as this is already
> +			       * found by flex and stored in yyleng
> +			       */
> +			      yylval->identifier = (char *) linear_alloc_child(ctx, yyleng + 1);
> +			      memcpy(yylval->identifier, yytext, yyleng + 1);
>   			    }
>   			    return classify_identifier(state, yytext);
>   			}
> diff --git a/src/compiler/glsl/glsl_parser.yy b/src/compiler/glsl/glsl_parser.yy
> index 7b93d34fa3..30b04beb4d 100644
> --- a/src/compiler/glsl/glsl_parser.yy
> +++ b/src/compiler/glsl/glsl_parser.yy
> @@ -100,7 +100,7 @@ static bool match_layout_qualifier(const char *s1, const char *s2,
>      int64_t n64;
>      float real;
>      double dreal;
> -   const char *identifier;
> +   char *identifier;
>   
>      struct ast_type_qualifier type_qualifier;
>   
> 


More information about the mesa-dev mailing list