[swift-evolution] [swift-evolution-announce] [Review] SE-0184: Unsafe[Mutable][Raw][Buffer]Pointer: add missing methods, adjust existing labels for clarity, and remove deallocation size

[Taylor Swift]

I don’t see any source for this claim in the documentation
<https://developer.apple.com/documentation/swift/unsafemutablepointer/2295090-deallocate>,
or the source code
<https://github.com/apple/swift/blob/master/stdlib/public/core/UnsafePointer.swift.gyb#L432>.
As far as I can tell the expected behavior is that partial deallocation
“should” work.

On Thu, Sep 7, 2017 at 8:59 AM, Joe Groff <jgroff at apple.com> wrote:

>
> > On Sep 6, 2017, at 4:46 PM, Taylor Swift <kelvin13ma at gmail.com> wrote:
> >
> >
> >
> >> On Sep 6, 2017, at 5:12 PM, Joe Groff <jgroff at apple.com> wrote:
> >>
> >>
> >>> On Sep 6, 2017, at 3:07 PM, Taylor Swift <kelvin13ma at gmail.com> wrote:
> >>>
> >>>
> >>>
> >>>> On Sep 6, 2017, at 4:31 PM, Joe Groff <jgroff at apple.com> wrote:
> >>>>
> >>>>
> >>>>> On Sep 6, 2017, at 2:28 PM, Andrew Trick <atrick at apple.com> wrote:
> >>>>>
> >>>>>
> >>>>>>> On Sep 6, 2017, at 1:12 PM, Joe Groff via swift-evolution <
> swift-evolution at swift.org> wrote:
> >>>>>>>
> >>>>>>>
> >>>>>>> On Sep 6, 2017, at 1:06 PM, Taylor Swift <kelvin13ma at gmail.com>
> wrote:
> >>>>>>>
> >>>>>>>
> >>>>>>>
> >>>>>>>> On Wed, Sep 6, 2017 at 12:41 PM, Joe Groff via swift-evolution <
> swift-evolution at swift.org> wrote:
> >>>>>>>> Currently, memory is deallocated by an instance method on
> UnsafeMutablePointer, deallocate(count:). Like much of the Swift pointer
> API, performing this operation on a buffer pointer requires extracting
> baseAddress! and count. It is very common for the allocation code above to
> be immediately followed by:
> >>>>>>>>
> >>>>>>>> defer
> >>>>>>>>
> >>>>>>>> {
> >>>>>>>> buffer.
> >>>>>>>> baseAddress?.deallocate(capacity: buffer.count
> >>>>>>>> )
> >>>>>>>> }
> >>>>>>>>
> >>>>>>>> This method is extremely problematic because nearly all users, on
> first seeing the signature of deallocate(capacity:), will naturally
> conclude from the capacity label that deallocate(capacity:) is equivalent
> to some kind of realloc()that can only shrink the buffer. However this is
> not the actual behavior — deallocate(capacity:) actually ignores the
> capacity argument and just calls free() on self. The current API is not
> only awkward and suboptimal, it is misleading. You can write perfectly
> legal Swift code that shouldn’t segfault, but still can, for example
> >>>>>>>>
> >>>>>>>> var ptr = UnsafeMutablePointer<UInt8>.allocate(capacity: 1000000
> >>>>>>>> )
> >>>>>>>> ptr.
> >>>>>>>> initialize(to: 13, count: 1000000
> >>>>>>>> )
> >>>>>>>> ptr.
> >>>>>>>> deallocate(capacity: 500000) // deallocate the second half of the
> memory block
> >>>>>>>> ptr[0] // segmentation fault
> >>>>>>>> where the first 500000 addresses should still be valid if the
> documentation is to be read literally.
> >>>>>>>
> >>>>>>> The fact that the Swift runtime currently uses malloc/free is an
> implementation detail. Tracking deallocation size is a very useful
> optimization for better allocator backends, and C++ underwent an ABI break
> to make it so that sized delete can be supported. Maybe we can change the
> name to `deallocate(allocatedCapacity:)` to make it clear that it isn't
> resizing the memory, and/or make the capacity argument optional so that you
> can pay for the overhead of the allocator deriving the size if it's
> inconvenient for the calling code to carry the size around, but we
> shouldn't remove the functionality altogether.
> >>>>>>>
> >>>>>>> -Joe
> >>>>>>> _______________________________________________
> >>>>>>> swift-evolution mailing list
> >>>>>>> swift-evolution at swift.org
> >>>>>>> https://lists.swift.org/mailman/listinfo/swift-evolution
> >>>>>>>
> >>>>>>> The idea is to get the house in order by removing all parameters
> from deallocate(), since that’s what it really does right now. Then, in the
> future, if Swift gets a more sophisticated allocator backend, a new method
> like deallocate(capacity:) or reallocate(toCapacity:) could be added
> without conflicting with the currently named deallocate(capacity:).
> However, using the function signature to pretend that it does something it
> can’t actually do right now is extremely dangerous.
> >>>>>>
> >>>>>> I don't think that's a good idea in this case, because it's not
> unlikely we would explore an optimized allocator soon after ABI stability,
> and retrofitting these interfaces in a future version of Swift would put a
> deployment target limit on when they can be used, and mean that a lot of
> user code would need to be retrofitted to carry allocated capacities where
> needed to see any benefit.
> >>>>>>
> >>>>>> -Joe
> >>>>>
> >>>>> The fact that we’re using malloc and free is already part of the ABI
> because old libraries need to be able to deallocate memory allocated by
> newer libraries.
> >>>>
> >>>> The compiler doesn't ever generate calls directly to malloc and free,
> and the runtime entry points we do use already take size and alignment on
> both allocation and deallocation.
> >>>>
> >>>>> Within the standard library we could make use of some new
> deallocation fast path in the future without worrying about backward
> deployment.
> >>>>>
> >>>>> Outside of the standard library, clients will get the benefits of
> whatever allocator is available on their deployed platform because we now
> encourage them to use UnsafeBufferPointer.deallocate(). We can change the
> implementation inside UnsafeBufferPointer all we want, as long as it’s
> still malloc-compatible.
> >>>>>
> >>>>> I’m sure we’ll want to provide a better allocation/deallocation API
> in the future for systems programmers based on move-only types. That will
> already be deployment-limited.
> >>>>>
> >>>>> Absolute worst case, we introduce a sized UnsafePointer.deallocate
> in the future. Any new code outside the stdlib that wants the performance
> advantage would either need to
> >>>>> - trivially wrap deallocation using UnsafeBufferPointer
> >>>>> - create a trivial UnsafePointer.deallocate thunk under an
> availability flag
> >>>>
> >>>> Since we already have sized deallocate, why would we take it away? If
> the name is misleading, we can change the name.
> >>>>
> >>>> -Joe
> >>>
> >>> Exactly, we are changing the name to `deallocate()`. As for the old
> `deallocate(capacity:)` method that *needs* to be removed because it is
> actively harmful. As I’ve explained it’s not enough to just drop in a sized
> backend later as an “implementation detail” because it’s not an
> implementation detail, you’re changing the fundamental behavior of that
> method.
> >>
> >> It would remove a mode of "holding it wrong", but the specified
> behavior will not change; it has and will always fully deallocate the
> object being referenced in the cases where the call has defined behavior.
> >
> > idk what you mean by it holding it wrong, but  while the specified
> behavior won’t change the actual behavior *will* change, because right now
> the two don’t agree. We can’t just treat it as a bug to fix because it’s
> been around since the beginning of Swift and some people treat it as part
> of expected behavior, using the function like `free()`. This isn’t using it
> incorrectly, in fact in terms of behavior, passing a meaningful capacity
> argument to `deallocate(capacity:)` is *incorrect usage*, as demonstrated
> in the segfaulting example in the proposal.
>
> The segfaulting example is an incorrect usage. The only valid parameters
> to deallocate(capacity:) are the base address of an allocation, and the
> original capacity passed into allocate(); it has never been intended to
> support partial deallocation of allocated blocks. It seems to me like this
> proposal is based on a misunderstanding of how the API works. The
> documentation and/or name should be clarified.
>
> -Joe
>
> > “fixing” this bug will cause programs that once operated on previously
> valid assumptions of “free()” semantics to behave differently, without any
> warnings ever being generated. Conversely incorrect code will suddenly
> become “correct” though this is less of a problem.
> >
> >> A sized implementation may fail more obviously when you violate the
> contract in the future. Not having sized deallocation is a known deficiency
> of the C model we've been fairly diligent about avoiding in Swift's
> allocation interfaces, and it would be extremely unfortunate for us to
> backpedal from it.
> >>
> >> -Joe
> >
> > Which is worse, an active gaping hole in Swift’s memory system, or
> potentially discouraging users from using a hypothetical future allocation
> API?
> >
> > Making sure the existing allocation API is working properly is a
> prerequisite to introducing a future more advanced allocation API.
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.swift.org/pipermail/swift-evolution/attachments/20170907/c67528b6/attachment.html>