fix shrink_in_place implementation

[gnzlbg]

This PR fixes a bug in the shrink_in_place implementation: if the size of the shrunk memory block is greater or equal than the desired size it currently errors, but this should actually be a success, and the else branch cannot ever happen unless there is a bug in jemalloc.

That is, the user asks for a desired size that is smaller than the original allocation size, and we are allowed to return a memory block that is larger than that.

[gnzlbg]

I've moved my thoughts to the Alloc trait issue: rust-lang/rust#42774 (comment)

[pnkfelix]

Hmm. I had assumed the size class of the shrunk value could be in a different bin (where the bins are arranged by size class) and the subsequent deallocation breaks ... in fact I could have sworn someone demonstrated a case of this to me using jemalloc ...

[gnzlbg]

I asked for clarification here (jemalloc/jemalloc#1222):

For small allocations, since they are slab managed, even shrinking will fail unless the new size falls into the same size class. When not resized, xallocx returns the old size.

By fail they mean "the allocation cannot be shrunk" so xallocx returns the usable_size of the allocation which for us means "success" because the API of shrink_to_fit is broken: as specified it is "shrink as much as possible" which is something that can never fail, since shrinking the allocation by zero is ok - hell, even returning a larger size is ok (and that will happen with jemalloc if the usable size is larger than the one in Layout).

We should change this so that shrink_to_fit returns (), and ideally we should make it return usize for the "excess".

[pnkfelix]

as specified it is "shrink as much as possible" which is something that can never fail, since shrinking the allocation by zero is ok

I don't understand how you are drawing that conclusion.

Its possible that the documentation is misleading (the definition of the "fits" predicate has been an ongoing problem for the Alloc trait), but my understanding of the design for shrink_to_fit is that when it returns Ok, the client is allowed (and actually, expected) to subsequently call dealloc using a layout that uses the new_size.

And you cannot assume that such a dealloc call will always succeed, because it could map to a differently sized bin. (I think on jemalloc this then causes a memory leak, or at least that was the behavior circa 2015.)

• This was in particular what motivated adding a lower-bound to the return type of fn usable_size, which was motivated by the short discussion on the Allocator RFC starting here: Allocators, take III rust-lang/rfcs#1398 (comment)

That is, it is not "shrink as much as you can, attempting to reach new_size". It is: "shrink to new_size. If the block cannot be soundly reclassified as having new_size, then return Err."