[PATCH] D27003: Return memory to OS right after free (not in the async thread).

[Evgeniy Stepanov via llvm-commits]

eugenis added inline comments.


================
Comment at: lib/asan/asan_allocator.cc:296
     allocator.SetMayReturnNull(options.may_return_null);
+    allocator.SetReleaseToOSIntervalMs(options.release_to_os_interval_ms);
     SharedInitCode(options);
----------------
This function is only used in asan_activation.cc, but you are not adding the flag to asan_activation_flags.inc...
Please add the flag and test it (see TestCases/Linux/activation-options.cc).


================
Comment at: lib/sanitizer_common/sanitizer_allocator_primary64.h:87
+  u32 ReleaseToOSIntervalMs() const {
+    return atomic_load(&release_to_os_interval_ms_, memory_order_acquire);
+  }
----------------
I think this can be memory_order_relaxed.


================
Comment at: lib/sanitizer_common/sanitizer_allocator_primary64.h:488
+      return;  // No chance to release anything.
     if ((region->rtoi.n_freed_at_last_release - region->n_freed) * chunk_size <
+        kReleaseToOsGranularity) {
----------------
Wait, is this not always negative / underflown?
region->n_freed grows monotonically.



================
Comment at: lib/sanitizer_common/sanitizer_allocator_primary64.h:498
+    u64 now_ns = NanoTime();
+    if (region->rtoi.last_release_at_ns + interval_ms * 1000000 > now_ns)
+      return;  // Memory was returned recently.
----------------
For general sanity, check that now_ns is larger than last_release_at_ns.


================
Comment at: lib/sanitizer_common/sanitizer_flag_parser.h:71
 template <>
+inline bool FlagHandler<unsigned int>::Parse(const char *value) {
+  char *value_end;
----------------
Just make the flag signed int, and the special "never" value - (-1).


https://reviews.llvm.org/D27003