modocache added a reviewer: vsk. modocache planned changes to this revision. modocache added a comment.
Thanks for the feedback, @vsk, I really appreciate it! I have some other work done for this on my local checkout, but I was going a little bonkers working on it without knowing whether people would want it merged or not. I'll update this with your feedback and upload the rest for review as well. > It'd be nice to dump this timer from Preprocessor::PrintStats(), too. Good idea, will do! ================ Comment at: lib/Lex/Preprocessor.cpp:660 + llvm::TimeRegion(PPOpts->ShowTimers ? &PreprocessingTimer : nullptr); + ---------------- vsk wrote: > vsk wrote: > > I wonder whether this is too fine-grained. I think setting up a timer in > > Preprocessor::Lex() might capture more information. Would you mind > > experimenting with that? > Nitpick: it may be useful to add PPOpts::getTimer(), in case we find more > sites where we need to either get back the PP timer or nullptr. > I think setting up a timer in `Preprocessor::Lex()` might capture more > information. Yes, can do! As it happens, the timers in this current diff cover all of the event counters that are incremented and then printed in `Preprocessor::PrintStats()`, except two: `NumTokenPaste` and `NumFastTokenPaste`. Beginning the timer in the Lexer would allow me to measure the time it takes to do token pasting as well, so I think this is a great idea. Thanks! > it may be useful to add `PPOpts::getTimer()` Agreed, will do! https://reviews.llvm.org/D36492 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
