Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[SR-9980] Eliminate undesired heap allocation of closure box in deinit function of ManagedBuffer subclass #52384

Closed
swift-ci opened this issue Feb 23, 2019 · 3 comments
Labels
compiler The Swift compiler in itself improvement standard library Area: Standard library umbrella

Comments

@swift-ci
Copy link
Collaborator

Previous ID SR-9980
Radar None
Original Reporter mattgallagher (JIRA User)
Type Improvement
Status Resolved
Resolution Done

Attachment: Download

Environment

Apple Swift version 5.0 (swiftlang-1001.0.63.8 clang-1001.0.43)

Target: x86_64-apple-darwin18.2.0

Running on MacBookPro15,1 Mojave 10.14.3 (18D109)

Additional Detail from JIRA
Votes 0
Component/s Compiler, Standard Library
Labels Improvement
Assignee None
Priority Medium

md5: 256534986a9305eb7b3574fe98003ca7

Issue Description:

When using Swift.ManagedBuffer to create a copy-on-write collection, it is not reasonably possible to avoid 2 additional allocations and deallocations when cleaning up the contents of the managed buffer in the deinit function. Correct use of Swift.ManagedBuffer requires a call to withUnsafeMutablePointers in the deinit function (to deinitialize contents) and the closure passed to withUnsafeMutablePointers is never optimized to the stack in the deinit function (despite this optimization working elsewhere) resulting in two additional allocations and deallocations and an approximate tripling of the time taken for micro tests involving Swift.ManagedBuffer subclasses.

I've attached a main.swift file that replicates the problem. Note that the problem relates to the `deinit` function between lines 465 and 482.

Compiling using `swiftc -g -O main.swift` and running through Instruments gives 974ms in _swift_release_dealloc

Change the #if conditions at lines 447 and 466 from false to true and Instruments gives 310ms in _swift_release_dealloc

The bug here is that the two code paths should be equivalent but Swift is unable to perform needed stack optimization.

This code and the original description of the problem dates to 2016:

https://www.cocoawithlove.com/blog/2016/09/22/deque.html

but the problem remains in Swift 5 beta 3 and the numbers have not changed substantially since the original article in 2016.

@belkadan
Copy link
Contributor

cc @eeckstein

@eeckstein
Copy link
Member

This is fixed with #21933
But that change is not included in swift-5.0, yet.

@swift-ci
Copy link
Collaborator Author

Comment by Matt Gallagher (JIRA)

Oh, derp. Yeah, testing this with swiftc built from master, I can confirm that the desired behavior is already implemented.

@swift-ci swift-ci transferred this issue from apple/swift-issues Apr 25, 2022
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compiler The Swift compiler in itself improvement standard library Area: Standard library umbrella
Projects
None yet
Development

No branches or pull requests

3 participants