From: Ran Benita Date: Wed, 13 Nov 2019 20:41:38 +0000 (+0200) Subject: parser: fix quadratic pointer chasing X-Git-Tag: xkbcommon-0.10.0~49 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=7c42945e04a2107827a057245298dedc0475cc88;p=platform%2Fupstream%2Flibxkbcommon.git parser: fix quadratic pointer chasing In the AST, lists (e.g. the list of statements in a file) are kept in singly-linked lists -- each AST node has a `next` pointer available for this purpose. Previously, a node was added to the list by starting from the head, chasing to the last, and appending. So creating a list of length N would take ~N^2/2 pointer dereferences. Now, we always (temporarily) keep the last as well, so appending is O(1) instead of O(N). Given a keymap xkb_keymap { xkb_keycodes { minimum = 8; minimum = 8; minimum = 8; minimum = 8; minimum = 8; [... repeated N times ...] }; xkb_types {}; xkb_compat {}; xkb_symbols {}; }; The compilation times are N | Before | After --------|----------|------- 10,000 | 0.407s | 0.006s 20,000 | 1.851s | 0.015s 30,000 | 5.737s | 0.021s 40,000 | 12.759s | 0.023s 50,000 | 21.489s | 0.035s 60,000 | 40.473s | 0.041s 70,000 | 53.336s | 0.039s 80,000 | 72.485s | 0.044s 90,000 | 94.703s | 0.048s 100,000 | 118.390s | 0.057s Another option is to ditch the linked lists and use arrays instead. I got it to work, but its more involved and allocation heavy so turns out to be worse without further optimizations. Signed-off-by: Ran Benita --- diff --git a/src/xkbcomp/ast-build.c b/src/xkbcomp/ast-build.c index 64051c7..c2f095d 100644 --- a/src/xkbcomp/ast-build.c +++ b/src/xkbcomp/ast-build.c @@ -55,20 +55,6 @@ #include "ast-build.h" #include "include.h" -ParseCommon * -AppendStmt(ParseCommon *to, ParseCommon *append) -{ - ParseCommon *iter; - - if (!to) - return append; - - for (iter = to; iter->next; iter = iter->next); - - iter->next = append; - return to; -} - static ExprDef * ExprCreate(enum expr_op_type op, enum expr_value_type type, size_t size) { @@ -568,7 +554,7 @@ XkbFileFromComponents(struct xkb_context *ctx, enum xkb_file_type type; IncludeStmt *include = NULL; XkbFile *file = NULL; - ParseCommon *defs = NULL; + ParseCommon *defs = NULL, *defsLast = NULL; for (type = FIRST_KEYMAP_FILE_TYPE; type <= LAST_KEYMAP_FILE_TYPE; type++) { include = IncludeCreate(ctx, components[type], MERGE_DEFAULT); @@ -581,7 +567,10 @@ XkbFileFromComponents(struct xkb_context *ctx, goto err; } - defs = AppendStmt(defs, &file->common); + if (!defs) + defsLast = defs = &file->common; + else + defsLast = defsLast->next = &file->common; } file = XkbFileCreate(FILE_TYPE_KEYMAP, NULL, defs, 0); diff --git a/src/xkbcomp/ast-build.h b/src/xkbcomp/ast-build.h index 46c7fbb..3afb080 100644 --- a/src/xkbcomp/ast-build.h +++ b/src/xkbcomp/ast-build.h @@ -27,9 +27,6 @@ #ifndef XKBCOMP_AST_BUILD_H #define XKBCOMP_AST_BUILD_H -ParseCommon * -AppendStmt(ParseCommon *to, ParseCommon *append); - ExprDef * ExprCreateString(xkb_atom_t str); diff --git a/src/xkbcomp/parser.y b/src/xkbcomp/parser.y index 5a2f11e..9b16fc7 100644 --- a/src/xkbcomp/parser.y +++ b/src/xkbcomp/parser.y @@ -169,9 +169,13 @@ resolve_keysym(const char *name, xkb_keysym_t *sym_rtrn) enum xkb_map_flags mapFlags; xkb_keysym_t keysym; ParseCommon *any; + struct { ParseCommon *head; ParseCommon *last; } anyList; ExprDef *expr; + struct { ExprDef *head; ExprDef *last; } exprList; VarDef *var; + struct { VarDef *head; VarDef *last; } varList; VModDef *vmod; + struct { VModDef *head; VModDef *last; } vmodList; InterpDef *interp; KeyTypeDef *keyType; SymbolsDef *syms; @@ -183,6 +187,7 @@ resolve_keysym(const char *name, xkb_keysym_t *sym_rtrn) KeyAliasDef *keyAlias; void *geom; XkbFile *file; + struct { XkbFile *head; XkbFile *last; } fileList; } %type INTEGER FLOAT @@ -196,11 +201,15 @@ resolve_keysym(const char *name, xkb_keysym_t *sym_rtrn) %type MapName OptMapName %type FieldSpec Ident Element String %type KeySym -%type DeclList Decl -%type OptExprList ExprList Expr Term Lhs Terminal ArrayInit KeySyms -%type OptKeySymList KeySymList Action ActionList Coord CoordList -%type VarDecl VarDeclList SymbolsBody SymbolsVarDecl -%type VModDecl VModDefList VModDef +%type Decl +%type DeclList +%type Expr Term Lhs Terminal ArrayInit KeySyms +%type OptKeySymList KeySymList Action Coord CoordList +%type OptExprList ExprList ActionList +%type VarDecl SymbolsVarDecl +%type VarDeclList SymbolsBody +%type VModDef +%type VModDefList VModDecl %type InterpretDecl InterpretMatch %type KeyTypeDecl %type SymbolsDecl @@ -213,15 +222,19 @@ resolve_keysym(const char *name, xkb_keysym_t *sym_rtrn) %type ShapeDecl SectionDecl SectionBody SectionBodyItem RowBody RowBodyItem %type Keys Key OverlayDecl OverlayKeyList OverlayKey OutlineList OutlineInList %type DoodadDecl -%type XkbFile XkbMapConfigList XkbMapConfig +%type XkbFile XkbMapConfig +%type XkbMapConfigList %type XkbCompositeMap %destructor { FreeStmt((ParseCommon *) $$); } +%destructor { FreeStmt((ParseCommon *) $$.head); } + /* The destructor also runs on the start symbol when the parser *succeeds*. * The `if` here catches this case. */ %destructor { if (!param->rtrn) FreeXkbFile($$); } +%destructor { FreeXkbFile($$.head); } %destructor { free($$); } %% @@ -249,7 +262,7 @@ XkbFile : XkbCompositeMap XkbCompositeMap : OptFlags XkbCompositeType OptMapName OBRACE XkbMapConfigList CBRACE SEMI - { $$ = XkbFileCreate($2, $3, (ParseCommon *) $5, $1); } + { $$ = XkbFileCreate($2, $3, (ParseCommon *) $5.head, $1); } ; XkbCompositeType: XKB_KEYMAP { $$ = FILE_TYPE_KEYMAP; } @@ -258,17 +271,16 @@ XkbCompositeType: XKB_KEYMAP { $$ = FILE_TYPE_KEYMAP; } ; XkbMapConfigList : XkbMapConfigList XkbMapConfig - { $$ = (XkbFile *) AppendStmt((ParseCommon *) $1, - (ParseCommon *) $2); } + { $$.head = $1.head; $$.last->common.next = &$2->common; $$.last = $2; } | XkbMapConfig - { $$ = $1; } + { $$.head = $$.last = $1; } ; XkbMapConfig : OptFlags FileType OptMapName OBRACE DeclList CBRACE SEMI { - $$ = XkbFileCreate($2, $3, $5, $1); + $$ = XkbFileCreate($2, $3, $5.head, $1); } ; @@ -298,8 +310,27 @@ Flag : PARTIAL { $$ = MAP_IS_PARTIAL; } ; DeclList : DeclList Decl - { $$ = AppendStmt($1, $2); } - | { $$ = NULL; } + { + /* + * TODO: This is needed because of VModDecl, which + * is a list and is "inlined" into the DeclList. + * Should change to avoid the O(N)-append behavior, + * like we do in the other lists. + */ + if ($2) { + if (!$1.head) + $$.head = $2; + else + $$.head = $1.head; + if (!$1.last) + $$.last = $2; + else + $$.last = $1.last->next = $2; + while ($$.last->next) + $$.last = $$.last->next; + } + } + | { $$.head = $$.last = NULL; } ; Decl : OptMergeMode VarDecl @@ -309,9 +340,9 @@ Decl : OptMergeMode VarDecl } | OptMergeMode VModDecl { - for (VModDef *vmod = $2; vmod; vmod = (VModDef *) vmod->common.next) + for (VModDef *vmod = $2.head; vmod; vmod = (VModDef *) vmod->common.next) vmod->merge = $1; - $$ = (ParseCommon *) $2; + $$ = (ParseCommon *) $2.head; } | OptMergeMode InterpretDecl { @@ -389,10 +420,9 @@ VModDecl : VIRTUAL_MODS VModDefList SEMI ; VModDefList : VModDefList COMMA VModDef - { $$ = (VModDef *) AppendStmt((ParseCommon *) $1, - (ParseCommon *) $3); } + { $$.head = $1.head; $$.last->common.next = &$3->common; $$.last = $3; } | VModDef - { $$ = $1; } + { $$.head = $$.last = $1; } ; VModDef : Ident @@ -404,7 +434,7 @@ VModDef : Ident InterpretDecl : INTERPRET InterpretMatch OBRACE VarDeclList CBRACE SEMI - { $2->def = $4; $$ = $2; } + { $2->def = $4.head; $$ = $2; } ; InterpretMatch : KeySym PLUS Expr @@ -414,30 +444,28 @@ InterpretMatch : KeySym PLUS Expr ; VarDeclList : VarDeclList VarDecl - { $$ = (VarDef *) AppendStmt((ParseCommon *) $1, - (ParseCommon *) $2); } + { $$.head = $1.head; $$.last->common.next = &$2->common; $$.last = $2; } | VarDecl - { $$ = $1; } + { $$.head = $$.last = $1; } ; KeyTypeDecl : TYPE String OBRACE VarDeclList CBRACE SEMI - { $$ = KeyTypeCreate($2, $4); } + { $$ = KeyTypeCreate($2, $4.head); } ; SymbolsDecl : KEY KEYNAME OBRACE SymbolsBody CBRACE SEMI - { $$ = SymbolsCreate($2, $4); } + { $$ = SymbolsCreate($2, $4.head); } ; SymbolsBody : SymbolsBody COMMA SymbolsVarDecl - { $$ = (VarDef *) AppendStmt((ParseCommon *) $1, - (ParseCommon *) $3); } + { $$.head = $1.head; $$.last->common.next = &$3->common; $$.last = $3; } | SymbolsVarDecl - { $$ = $1; } - | { $$ = NULL; } + { $$.head = $$.last = $1; } + | { $$.head = $$.last = NULL; } ; SymbolsVarDecl : Lhs EQUALS Expr { $$ = VarCreate($1, $3); } @@ -450,7 +478,7 @@ SymbolsVarDecl : Lhs EQUALS Expr { $$ = VarCreate($1, $3); } ArrayInit : OBRACKET OptKeySymList CBRACKET { $$ = $2; } | OBRACKET ActionList CBRACKET - { $$ = ExprCreateActionList($2); } + { $$ = ExprCreateActionList($2.head); } ; GroupCompatDecl : GROUP Integer EQUALS Expr SEMI @@ -458,11 +486,11 @@ GroupCompatDecl : GROUP Integer EQUALS Expr SEMI ; ModMapDecl : MODIFIER_MAP Ident OBRACE ExprList CBRACE SEMI - { $$ = ModMapCreate($2, $4); } + { $$ = ModMapCreate($2, $4.head); } ; LedMapDecl: INDICATOR String OBRACE VarDeclList CBRACE SEMI - { $$ = LedMapCreate($2, $4); } + { $$ = LedMapCreate($2, $4.head); } ; LedNameDecl: INDICATOR Integer EQUALS Expr SEMI @@ -513,7 +541,7 @@ Keys : Keys COMMA Key { $$ = NULL; } Key : KEYNAME { $$ = NULL; } | OBRACE ExprList CBRACE - { FreeStmt((ParseCommon *) $2); $$ = NULL; } + { FreeStmt((ParseCommon *) $2.head); $$ = NULL; } ; OverlayDecl : OVERLAY String OBRACE OverlayKeyList CBRACE SEMI @@ -552,7 +580,7 @@ Coord : OBRACKET SignedNumber COMMA SignedNumber CBRACKET ; DoodadDecl : DoodadType String OBRACE VarDeclList CBRACE SEMI - { FreeStmt((ParseCommon *) $4); $$ = NULL; } + { FreeStmt((ParseCommon *) $4.head); $$ = NULL; } ; DoodadType : TEXT { $$ = 0; } @@ -608,14 +636,13 @@ MergeMode : INCLUDE { $$ = MERGE_DEFAULT; } ; OptExprList : ExprList { $$ = $1; } - | { $$ = NULL; } + | { $$.head = $$.last = NULL; } ; ExprList : ExprList COMMA Expr - { $$ = (ExprDef *) AppendStmt((ParseCommon *) $1, - (ParseCommon *) $3); } + { $$.head = $1.head; $$.last->common.next = &$3->common; $$.last = $3; } | Expr - { $$ = $1; } + { $$.head = $$.last = $1; } ; Expr : Expr DIVIDE Expr @@ -643,7 +670,7 @@ Term : MINUS Term | Lhs { $$ = $1; } | FieldSpec OPAREN OptExprList CPAREN %prec OPAREN - { $$ = ExprCreateAction($1, $3); } + { $$ = ExprCreateAction($1, $3.head); } | Terminal { $$ = $1; } | OPAREN Expr CPAREN @@ -651,14 +678,13 @@ Term : MINUS Term ; ActionList : ActionList COMMA Action - { $$ = (ExprDef *) AppendStmt((ParseCommon *) $1, - (ParseCommon *) $3); } + { $$.head = $1.head; $$.last->common.next = &$3->common; $$.last = $3; } | Action - { $$ = $1; } + { $$.head = $$.last = $1; } ; Action : FieldSpec OPAREN OptExprList CPAREN - { $$ = ExprCreateAction($1, $3); } + { $$ = ExprCreateAction($1, $3.head); } ; Lhs : FieldSpec