[profile] Decommit memory after counter relocation

[phosek]

Summary

After we relocate counters, we no longer need to keep the original copy
around so we can return the memory back to the operating system.

[vsk]

compiler-rt/lib/profile/InstrProfilingUtil.c

345

See InstrProfilingPort.h "getpagesize", this looks like it can replace that.

compiler-rt/lib/profile/InstrProfilingUtil.h

80

Should these helpers assert that boundary is a power of two (not sure what the clearest way would be to check -- maybe assert(popcnt(b) == 1))?

[davidxl]

compiler-rt/lib/profile/InstrProfilingFile.c

557

Is this portable across all platforms?

compiler-rt/lib/profile/InstrProfilingPlatformFuchsia.c

199

The snippet is very similar to the one in InstProfilingFile.c -- can they be commoned?

[mcgrathr]

Factoring out the arithmetic from the OS-specific code and landing only the Fuchsia version of the OS-specific implementation LGTM.

compiler-rt/lib/profile/InstrProfilingFile.c

557

Unlikely. It may work on some systems other than Linux, but you'd have to check.
Another option is posix_madvise, which is in theory more portable but again you'd have to check which systems actually have it, and its specified semantics are weaker than what you'd like to use here.

Because of the uncertainty of the portable solution for other platforms, I'd recommend that we just land a change affecting only Fuchsia first.

compiler-rt/lib/profile/InstrProfilingPlatformFuchsia.c

199

I agree it would be reasonable to consolidate this arithmetic into a call to an OS-specific function akin to sanitizer_common's ReleaseMemoryPagesToOS. That refactoring still fits well if you start out with only the Fuchsia implementation actually doing anything and leave the other ones as an empty TODO stub until the portability issues are worked out.

compiler-rt/lib/profile/InstrProfilingUtil.c

345

Agreed. There is also __llvm_profile_set_page_size, used only when a particular % magic sequence is found in an option setting (!). All that stuff should be cleaned up one way or another.

compiler-rt/lib/profile/InstrProfilingUtil.h

80

I don't think that's necessary. This kind of alignment rounding is common and always has the power-of-two constraint.

[phosek]

compiler-rt/lib/profile/InstrProfilingFile.c

557

I forgot about Windows and it turned out that implementing MADV_DONTNEED is straightforward so I did that.

[phosek]

Do you have any other comments? If not I'd like to go ahead and land this change.

[davidxl]

lgtm

compiler-rt/lib/profile/InstrProfilingFile.c

static void relocateCounters(void) {
   if (!__llvm_profile_is_continuous_mode_enabled() ||
       !lprofRuntimeCounterRelocation())
     return;
 
   /* Get the sizes of various profile data sections. Taken from
    * __llvm_profile_get_size_for_buffer(). */
   const __llvm_profile_data *DataBegin = __llvm_profile_begin_data();
   const __llvm_profile_data *DataEnd = __llvm_profile_end_data();
+  const uint64_t *CountersBegin = __llvm_profile_begin_counters();
+  const uint64_t *CountersEnd = __llvm_profile_end_counters();
   uint64_t DataSize = __llvm_profile_get_data_size(DataBegin, DataEnd);
   const uint64_t CountersOffset = sizeof(__llvm_profile_header) +
       (DataSize * sizeof(__llvm_profile_data));
 
   int Length = getCurFilenameLength();
   char *FilenameBuf = (char *)COMPILER_RT_ALLOCA(Length + 1);
   const char *Filename = getCurFilename(FilenameBuf, 0);
   if (!Filename)
if (!ProfileFileSize) {
         return;
       }
     }
 
     lprofUnlockFileHandle(File);
   }
 
   /* Update the profile fields based on the current mapping. */
-  __llvm_profile_counter_bias = (intptr_t)Profile -
-      (uintptr_t)__llvm_profile_begin_counters() + CountersOffset;
+  __llvm_profile_counter_bias =
+      (intptr_t)Profile - (uintptr_t)CountersBegin + CountersOffset;
+
+  /* Return the memory allocated for counters to OS. */
+  size_t PageSize = lprofGetPageSize();
+  uintptr_t CountersBeginAligned =
+      lprofRoundUpTo((uintptr_t)CountersBegin, PageSize);
+  uintptr_t CountersEndAligned =
+      lprofRoundDownTo((uintptr_t)CountersEnd, PageSize);
+  if (CountersBeginAligned < CountersEndAligned)
+    madvise((void *)CountersBeginAligned,
+            CountersEndAligned - CountersBeginAligned, MADV_DONTNEED);
 }
 
 static void initializeProfileForContinuousMode(void) {
   if (!__llvm_profile_is_continuous_mode_enabled())
     return;
 
 #if defined(__Fuchsia__) || defined(_WIN32)
   PROF_ERR("%s\n", "Continuous mode not yet supported on Fuchsia or Windows.");

compiler-rt/lib/profile/InstrProfilingPlatformFuchsia.c

void __llvm_profile_initialize(void) {
   /* Publish the VMO which contains profile data to the system. Note that this
    * also consumes the VMO handle. */
   __sanitizer_publish_data(ProfileSinkName, Vmo);
 
   /* Use the dumpfile symbolizer markup element to write the name of VMO. */
   lprofWrite("LLVM Profile: {{{dumpfile:%s:%s}}}\n", ProfileSinkName, VmoName);
 
   /* Update the profile fields based on the current mapping. */
-  __llvm_profile_counter_bias = (intptr_t)Mapping -
-                                (uintptr_t)__llvm_profile_begin_counters() +
-                                CountersOffset;
+  __llvm_profile_counter_bias =
+      (intptr_t)Mapping - (uintptr_t)CountersBegin + CountersOffset;
+
+  /* Return the memory allocated for counters to OS. */
+  size_t PageSize = lprofGetPageSize();
+  zx_vaddr_t CountersBeginAligned =
+      lprofRoundUpTo((uintptr_t)CountersBegin, PageSize);
+  zx_vaddr_t CountersEndAligned =
+      lprofRoundDownTo((uintptr_t)CountersEnd, PageSize);
+  if (CountersBeginAligned < CountersEndAligned) {
+    Status = _zx_vmar_op_range(
+        _zx_vmar_root_self(), ZX_VMAR_OP_DECOMMIT, CountersBeginAligned,
+        CountersEndAligned - CountersBeginAligned, NULL, 0);
+    if (Status != ZX_OK) {
+      lprofWrite("LLVM Profile: cannot decommit counters: %s\n",
+                 _zx_status_get_string(Status));
+      return;
+    }
+  }
 }
 
 #endif

compiler-rt/lib/profile/InstrProfilingUtil.h

 /*===- InstrProfilingUtil.h - Support library for PGO instrumentation -----===*\
 |*
 |* Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 |* See https://llvm.org/LICENSE.txt for license information.
 |* SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 |*
 \*===----------------------------------------------------------------------===*/
 
 #ifndef PROFILE_INSTRPROFILINGUTIL_H
 #define PROFILE_INSTRPROFILINGUTIL_H
 
+#include <inttypes.h>
 #include <stddef.h>
 #include <stdio.h>
 
 /*! \brief Create a directory tree. */
 void __llvm_profile_recursive_mkdir(char *Pathname);
 
 /*! Set the mode used when creating profile directories. */
 void __llvm_profile_set_dir_mode(unsigned Mode);

 /* Temporarily suspend SIGKILL. Return value of 1 means a restore is needed.
  * Other return values mean no restore is needed.
  */
 int lprofSuspendSigKill();
 
 /* Restore previously suspended SIGKILL. */
 void lprofRestoreSigKill();
 
+size_t lprofGetPageSize();
+
+inline size_t lprofRoundUpTo(size_t x, size_t boundary) {
+  return (x + boundary - 1) & ~(boundary - 1);
+}
+
+inline size_t lprofRoundDownTo(size_t x, size_t boundary) {
+  return x & ~(boundary - 1);
+}
+
 #endif /* PROFILE_INSTRPROFILINGUTIL_H */

compiler-rt/lib/profile/InstrProfilingUtil.c

 #include <stdlib.h>
 #include <string.h>
 
 #if defined(__linux__)
 #include <signal.h>
 #include <sys/prctl.h>
 #endif
 
+#if defined(__Fuchsia__)
+#include <zircon/syscalls.h>
+#endif
+
 #include "InstrProfiling.h"
 #include "InstrProfilingUtil.h"
 
 COMPILER_RT_WEAK unsigned lprofDirMode = 0755;
 
 COMPILER_RT_VISIBILITY
 void __llvm_profile_recursive_mkdir(char *path) {
   int i;

 #endif
 }
 
 COMPILER_RT_VISIBILITY void lprofRestoreSigKill() {
 #if defined(__linux__)
   prctl(PR_SET_PDEATHSIG, SIGKILL);
 #endif
 }
+
+COMPILER_RT_VISIBILITY size_t lprofGetPageSize() {
+#if defined(__Fuchsia__)
+  return _zx_system_get_page_size();
+#elif defined(__APPLE__) || \
+      defined(__linux__) || \
+      defined(__FreeBSD__) || \
+      defined(__NetBSD__)
+  return sysconf(_SC_PAGESIZE);  // EXEC_PAGESIZE may not be trustworthy.
+#elif defined(_WIN32)
+  SYSTEM_INFO si;
+  GetSystemInfo(&si);
+  return si.dwPageSize;
+#else
+  return 4096;
+#endif
+}