[PATCH] D39526: [XRay] Use optimistic logging model for FDR mode

[Martin Pelikán via Phabricator via llvm-commits]

pelikan added a comment.

still not 100% finished - I'll get to it tomorrow.  I am convinced the overall concept will work though.



================
Comment at: compiler-rt/lib/xray/xray_fdr_logging.cc:294
   std::memcpy(TLD.RecordPtr, Event, ReducedEventSize);
+  incrementExtents(MetadataRecSize + EventSize);
   endBufferIfFull();
----------------
pelikan wrote:
> I trust it's safe to incrementExtents after the memcpy(3) because this will never run concurrently in multiple threads.
It's correct because incrementExtents is acq_rel so before that technically nothing happened :-)  Marking as done.


================
Comment at: compiler-rt/lib/xray/xray_fdr_logging_impl.h:56
 /// Writes the new buffer record and wallclock time that begin a buffer for a
 /// thread to MemPtr and increments MemPtr. Bypasses the thread local state
 /// machine and writes directly to memory without checks.
----------------
You speak of MemPtr but your words mean nothing.


================
Comment at: compiler-rt/lib/xray/xray_fdr_logging_impl.h:195
+  auto &TLD = getThreadLocalData();
+  MetadataRecord Records[InitRecordsCount];
   {
----------------
Since we have different types of "records", how about calling the variable Metadata?


================
Comment at: compiler-rt/lib/xray/xray_fdr_logging_impl.h:316-317
 
-static inline void writeFunctionRecord(int FuncId, uint32_t TSCDelta,
-                                       XRayEntryType EntryType,
-                                       char *&MemPtr) XRAY_NEVER_INSTRUMENT {
+static inline void
+writeFunctionRecord(int FuncId, uint32_t TSCDelta,
+                    XRayEntryType EntryType) XRAY_NEVER_INSTRUMENT {
----------------
Plus million for this coding style.  Much better than wrapping in the middle of the screen.


https://reviews.llvm.org/D39526