[PATCH] D96626: Support: mapped_file_region: Pass MAP_NORESERVE to mmap

[Joseph Tremoulet via Phabricator via llvm-commits]

JosephTremoulet added a comment.

Ping.  I'd like to figure out how to make forward progress with this.  A couple points:

1 - Several commenters have mentioned they don't own this code and thus aren't the right people to officially approve/reject.  @sammccall, do you feel the same way, or are you comfortable making the call here?

2 - Which approach to take.  I see broadly four options:

option A - Never do the up-front reserve/commit check.  The failure mode in the OOM case isn't great, but neither will we artificially fail when we don't actually need that much physical memory (as we currently do with huge core files).  And it's at least consistent across platforms.  This is what the code in the patch currently does.

option B - Always do whatever is the host platform's default up-front reserve/commit check.  This is what the code in main currently does.  But on some Linux systems it requires users to adjust system settings to avoid those artificial failures, which is why I'm proposing a change.

option C - Parameterize mapped_file_region (and containing/calling utilities) to give callers the option of either doing no up-front reserve/commit check or doing whatever is the host platform's default up-front reserve/commit check.  My hesitation with this approach is that it adds complexity to the API for what seems like little benefit (and some false comfort at that, given the platform differences), and also that it has a bit of a "leaky abstraction code smell" in exposing a flag directly tied to a setting on certain host platforms in a utility that's supposed to provide a platform-agnostic interface.

option D - Parameterize mapped_file_region (and containing/calling utilities) to give callers the option of either doing no up-front reserve/commit check or doing some custom hand-written up-front reserve/commit check.  My hesitation with this approach is that it would be difficult to design a custom check that would work well without having artificial failures, and doubly difficult to do so when we don't have the actual use cases for that check in hand.  But if we decide that's The Right Thing long-term, then at least I'll have my answer.

Thoughts?

Thanks


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96626