[PATCH] D69586: [profile] Support online merging with continuous sync mode

[David Li via Phabricator via llvm-commits]

davidxl added inline comments.


================
Comment at: compiler-rt/lib/profile/InstrProfilingFile.c:400
+  /* By pass file truncation to allow online raw profile merging. */
+  if (lprofCurFilename.MergePoolSize)
+    return;
----------------
why this change?


================
Comment at: compiler-rt/lib/profile/InstrProfilingFile.c:496
+    uint64_t ProfileFileSize;
+    if (getProfileFileSizeForMerging(File, &ProfileFileSize) == -1)
+      goto unlockAndReturn;
----------------
perhaps:

int ret = getProfileFIleSizeForMerging(...);

if (ret == -1 || profileFileSize == 0) {

    // full write ...
} else {
    // merge
 }


================
Comment at: compiler-rt/lib/profile/InstrProfilingFile.c:529
    * can succeed. Leak the file handle, as the file should stay open. */
-  setProfileFile(File);
-  int rc = writeFile(Filename);
-  if (rc)
-    PROF_ERR("Failed to write file \"%s\": %s\n", Filename, strerror(errno));
-  setProfileFile(NULL);
+  if (ProfileRequiresFullWrite) {
+    setProfileFile(File);
----------------
Can this code be moved up to line 499 ? Is ProfileRequiresFullWrite flag still needed?


================
Comment at: compiler-rt/lib/profile/InstrProfilingFile.c:553
+
+unlockAndReturn:
+  if (ProfileRequiresUnlock) {
----------------
This can be hoisted closer to the merge handling.


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

https://reviews.llvm.org/D69586