Use MallocAllocator for unique_ptr on macOS.

[rnlahaye]

This is the second version (replacing #2901) of the second part of the fix for #2899.

This change makes the deleter for std::unique_ptr on macOS default to AllocatorDeleter, which uses an allocator for deletion (using the same principia::std approach as in #2900). The default allocator is std::allocator, which aliases to MallocAllocator on macOS as of #2900. A corresponding std::make_unique is also provided.
The danger with this approach is that if you have code of the form std::unique_ptr<Foo>(new Foo()), it will cause undefined behavior when the pointer is deleted.

To avoid this UB, I set AllocatorDeleter::pointer to a wrapper class which implicitly converts to a T* but must be explicitly constructed from a T*. Consequently, to construct a unique pointer from a raw pointer you have to write std::unique_ptr<Foo>(AssertAllocatedBy<std::allocator<Foo>(...)). The code snippet from the previous paragraph would cause a compile error (albeit only on macOS). For the cases when new is necessary (private constructors), I provide a tagged placement new as suggested by @eggrobin. It is used like this: new (AllocateWith<std::allocator<Foo>>{}) Foo(…). Thus, the snippet from the previous paragraph would be replaced by

std::unique_ptr<Foo>(AssertAllocatedBy<std::allocator<Foo>(new (AllocateWith<std::allocator<Foo>>{}) Foo()))

This is somewhat wordy but the intention is clear (the verbosity could probably be reduced with better naming).

Unfortunately, this change is not as seamless as #2900. It requires code changes in the following places:

• Use of new in factory functions for classes with private constructors (bb1a411; 7 files). There isn't really any way around this; the existing new calls are what we are trying to remove. The code changes to the verbose form shown above.
• Use of new in tests for convenience (3fc4823; 7 files). Changed to use std::make_unique.
• not_null assumes std::unique_ptr<T>::pointer is T* and requires some fixes (ce6db1a; 3 files).
• There are a few owned raw pointers in boundary code. AssertAllocatedBy is required to turn them into unique pointers (
  c44f898; 3 files).
• Sometimes a unique_ptr is constructed from another unique_ptr in an unusual way (a3b2611 and 4d3d18b; 2 files).

Additionally, I automatically fall back to ::std::unique_ptr for the following types:

• Arrays (as in T[], not std::array). The allocator deallocate function require the number of elements but that is not available to the deleter (neither ::std::allocator nor MallocAllocator actually use this parameter AFAIK, but I still don't want to break the contract).
• T where std::unique_ptr<T> is constructed by non-Principia code. This is just one class (google::compression::Compressor) so I special case it.

[rnlahaye]

Update: it crashes on load. It turns out that there are still more news that were not caught at compile time. Specifically, cases where an object is constructed and the raw pointer is returned directly (this happens at the interface boundary). I'm not sure how to catch these cases statically.

I changed all the cases of this I could find to use std::make_unique or allocator new, and it hasn't crashed yet. Of course, I could have missed some bad allocs and simply not encountered them yet.

At this point I don't think this change is feasible as it requires intrusive changes yet does not prevent accidental UB.

[pleroy]

We'll cut the next release, Goldbach, some time this week. The container changes, which I believe are safe, will go in that release. Do you have a sense of how much they gain?

As for this PR, let's keep it open for the time being. I want to review it carefully and see if/how it's possible to make things safer and/or less intrusive.

[rnlahaye]

I made some measurements of how much time per Unity FixedUpdate frame was spent in Principia code (note that this measurement does not cover background threads).

Version	Idle (KSC)	Idle (map)	100 000× warp (KSC)	Flight planning (map)
Pre-#2900 (669433e)	~10ms	~50ms	~500ms	~150ms
Post-#2900 (134ba85)	~3.6ms	~10ms	~10ms	~15ms
This change (e680990)	~0.6ms	~8ms	~10ms	~8ms

So, the gains vary from ~3× at idle to ~50× at high warp. The container change is definitely the more important of the two changes.

[eggrobin]

At this point I don't think this change is feasible as it requires intrusive changes yet does not prevent accidental UB.

This was probably a good assessment.

I want to review it carefully and see if/how it's possible to make things safer and/or less intrusive.

We have not found any way to be smart about it, and your numbers show that #2900 was what mattered anyway.

Closing this.