broadwaylamb marked an inline comment as done.
broadwaylamb added inline comments.


================
Comment at: llvm/lib/Support/TimeProfiler.cpp:266
+      const auto BeginningOfTimeUs = SystemTime - ProcessLocalTime;
+      J.attribute("beginningOfTime",
+                  BeginningOfTimeUs.time_since_epoch().count());
----------------
MaskRay wrote:
> broadwaylamb wrote:
> > broadwaylamb wrote:
> > > MaskRay wrote:
> > > > What is the logic here?
> > > > 
> > > > Two events in the same process may have different `beginningOfTime`. Is 
> > > > that intended?
> > > > 
> > > > Noe that `system_clock::now()` and `steady_clock::now()` are not 
> > > > measured at the same time.
> > > >Two events in the same process may have different beginningOfTime. Is 
> > > >that intended?
> > > 
> > > How would that be possible? We only compute `beginningOfTime` when 
> > > writing the data of the time profiler to a JSON file. AFAICT it happens 
> > > only once. So, this code is only called once.
> > > 
> > > >Noe that system_clock::now() and steady_clock::now() are not measured at 
> > > >the same time.
> > > I understand. But the difference should be very small (certainly less 
> > > than a microsecond), since there are only a few arithmetic operations in 
> > > between.
> > Although we could set the beginning of time in `TimeProfiler`'s constructor 
> > and assign it to a field. That would be a lot simpler, I guess. What do you 
> > think?
> OK, but I am still confused by the logic and the comment. Does the code just 
> want to compute the start time in system_clock?
Exactly, yes.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D78030/new/

https://reviews.llvm.org/D78030



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to