[clang] [time-trace] Add a new time trace scope variable named "ParseDeclarationOrFunctionDefinition". (PR #65268)
Aaron Ballman via cfe-commits
cfe-commits at lists.llvm.org
Tue Oct 31 05:30:29 PDT 2023
AaronBallman wrote:
> Hi @nikic, thanks for spotting the issue and reverted the commit.
>
> Hi @AaronBallman,
>
> As @nikic mentioned that the issue is that the used strings may be expensive to compute. If we remove the change to record the function name and function location, I think the issue will be fixed.
>
> This means:
>
> 1. Remove the time trace scope variable of `ParseFunctionDefinition`.
>
>
> ```
> llvm::TimeTraceScope TimeScope(
> "ParseFunctionDefinition",
> Actions.GetNameForDeclarator(D).getName().getAsString());
> ```
>
> 2. Remove the source location for the time trace scope variable of "ParseDeclarationOrFunctionDefinition".
> Change:
>
>
> ```
> llvm::TimeTraceScope TimeScope(
> "ParseDeclarationOrFunctionDefinition",
> Tok.getLocation().printToString(
> Actions.getASTContext().getSourceManager()));
> ```
>
> To:
>
> ```
> llvm::TimeTraceScope TimeScope(
> "ParseDeclarationOrFunctionDefinition");
> ```
>
> Could you please let me know your thoughts? or any suggestion?
>
> Thanks, Maggie
I think the suggestion from @nikic is to use the `TimeTraceScope` constructor accepting a function reference instead of the one accepting a string. e.g., change to using something like:
```
llvm::TimeTraceScope TimeScope(
"ParseDeclarationOrFunctionDefinition", [&] {
return Tok.getLocation().printToString(
Actions.getASTContext().getSourceManager())
});
```
https://github.com/llvm/llvm-project/pull/65268
More information about the cfe-commits
mailing list