MatzeB added inline comments.
================
Comment at: lib/Lex/Preprocessor.cpp:746
void Preprocessor::Lex(Token &Result) {
+ llvm::TimeRegion(PPOpts->getTimer());
+
----------------
modocache wrote:
> erik.pilkington wrote:
> > Doesn't this just start a timer and immediately end the timer? Shouldn't we
> > do: `llvm::TimeRegion LexTime(PPOpts->getTimer())` so that the dtor gets
> > called when this function returns and we track the time spent in this
> > function?
> >
> > Also: this is a pretty hot function, and it looks like TimeRegion does some
> > non-trivial work if time is being tracked. Have you tried testing this on a
> > big c++ file with and without this patch and seeing what the difference in
> > compile time looks like?
> Ah, yes you're right! Sorry about that. Actually keeping the timer alive for
> the duration of the method also reveals that the method is called
> recursively, which causes an assert, because timers can't be started twice.
>
> Another spot in Clang works around this with a "reference counted" timer:
> https://github.com/llvm-mirror/clang/blob/6ac9c51ede0a50cca13dd4ac03562c036f7a3f48/lib/CodeGen/CodeGenAction.cpp#L130-L134.
> I have a more generic version of this "reference counting timer" that I've
> been using for some of the other timers I've been adding; maybe I'll use it
> here as well.
FWIF: I share Eriks concerns about compiletime. Timers are enabled in optimized
builds, and querying them is not free. So putting one into a function that is
called a lot and is time critical seems like a bad idea (do benchmarking to
prove or disprove this!).
https://reviews.llvm.org/D36492
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
