perf: skip discarded syntax AST rebuild - #257
Conversation
Keep rebuildAst(true) as the full replacement-tree path, while rebuildAst(false) validates woven sources through clang-dumper without decoding its output into a temporary Clava AST.
Update the diagnostics-only ClangAstDumper construction for the rebased constructor signature.
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 992a85ec4c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| Standard standard = getStandard(sources, options); | ||
| ClangResources clangResources = new ClangResources(this); | ||
| var clangFiles = clangResources.getClangFiles(get(ClangAstKeys.LIBC_CXX_MODE)); |
There was a problem hiding this comment.
When LIBC_CXX_MODE is SYSTEM (or AUTO resolves to system), clangFiles.builtinIncludes() is empty, but options still carries toDataStore()'s default BUILTIN_AND_LIBC; ClangAstDumper consequently adds -nostdinc and -nostdinc++, so syntax checking rejects otherwise valid files that include system headers. The normal parse path avoids this by setting options to clangFiles.libcMode() immediately after resolving resources; validation needs the same assignment.
Useful? React with 👍 / 👎.
Replace the parallel validateSyntax orchestration chain (abstract method on CodeParser plus duplicated setup in ParallelCodeParser) with a SYNTAX_ONLY flag consulted by the existing parse() path, so clang arguments, resources, parallelism, and error handling have a single source of truth. ClangAstDumper.validateSyntax now returns the error message directly.
|
1 participant
Clava's
rebuildAst(false)path validated rewritten source by running clang/clang-dumper and then decoding the complete AST even though the rebuilt tree was immediately discarded.This adds a diagnostics-only validation path: it invokes the same clang/clang-dumper pipeline, drains stdout without AST decoding, preserves stderr and exit-status handling, and leaves
rebuildAst(true)on the full-parse path. The validator also receives the system resource directory required by the current clang setup.Measured impact: six paired runs show a median execution-time reduction of 11.1% (14.02 s); the mean reduction is 13.2%. The paired 95% confidence interval for the saving is 9.42–23.42 s (p = 0.0018). Full benchmark report.
Verification:
gradle --project-dir ClavaWeaver clean installDistgit diff --checkModel: Codex (GPT-5.6-luna).