[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