[Mesa-dev] [PATCH] glcpp: Avoid unnecessary call to strlen
Thomas Helland
thomashelland90 at gmail.com
Thu Aug 31 05:08:58 UTC 2017
2017-08-31 2:20 GMT+02:00 Timothy Arceri <tarceri at itsqueeze.com>:
>
>
> 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
>
Huh.. That is wierd. I'll do some more benchmarks
this evening to see if I can get a more statistically
sound result. ONe difference between my run and
yours is that I ran it in multithreaded mode.
I believe I built it with debug symbols, but I for sure
disabled NIR validation. I'll dig into this.
>
>> ---
>> 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