> On Sep 6, 2017, at 4:46 PM, Taylor Swift <kelvin1...@gmail.com> wrote:
>
>
>
>> On Sep 6, 2017, at 5:12 PM, Joe Groff <jgr...@apple.com> wrote:
>>
>>
>>> On Sep 6, 2017, at 3:07 PM, Taylor Swift <kelvin1...@gmail.com> wrote:
>>>
>>>
>>>
>>>> On Sep 6, 2017, at 4:31 PM, Joe Groff <jgr...@apple.com> wrote:
>>>>
>>>>
>>>>> On Sep 6, 2017, at 2:28 PM, Andrew Trick <atr...@apple.com> wrote:
>>>>>
>>>>>
>>>>>>> On Sep 6, 2017, at 1:12 PM, Joe Groff via swift-evolution
>>>>>>> <swift-evolution@swift.org> wrote:
>>>>>>>
>>>>>>>
>>>>>>> On Sep 6, 2017, at 1:06 PM, Taylor Swift <kelvin1...@gmail.com> wrote:
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>> On Wed, Sep 6, 2017 at 12:41 PM, Joe Groff via swift-evolution
>>>>>>>> <swift-evolution@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@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.
_______________________________________________
swift-evolution mailing list
swift-evolution@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution
