[PATCH] D116366: [Support] Add MemoryBuffer::dontNeedIfMmap

[Alexandre Ganea via Phabricator via llvm-commits]

aganea accepted this revision.
aganea added a comment.
This revision is now accepted and ready to land.

LGTM.



================
Comment at: llvm/lib/Support/Unix/Path.inc:874
+void mapped_file_region::dontNeedImpl() {
+  if (Mapping)
+    ::madvise(Mapping, Size, MADV_DONTNEED);
----------------
Since `MADV_DONTNEED` will drop all written changes to the memory pages, is it worth adding `assert(Mode == mapped_file_region::readonly);` here?


================
Comment at: llvm/lib/Support/Windows/Path.inc:962
 
+void mapped_file_region::dontNeedImpl() {}
+
----------------
Probably the closest we could do on Windows is:
```
void mapped_file_region::dontNeedImpl() {
  assert(Mode == mapped_file_region::readonly);
  VirtualFree(Mapping, Size, MEM_DECOMMIT);
}
```
...but that's "destructive", since `VirtualAlloc(..MEM_COMMIT..)` would be required to access the pages again. That would require another function `mapped_file_region::needImpl() ...`

There's also `VirtualAlloc(..MEM_RESET..)` which is closer to `MADV_DONTNEED` but that doesn't work with mmap'd files.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116366