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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to