Draft: Add @noescape and rethrows to ManagedBuffer API


(Karoy Lorentey) #1

I’d like to request feedback on the draft proposal below, about a (hopefully) trivial change to the standard library.

I considered adding it to SE-0012 <https://github.com/apple/swift-evolution/blob/master/proposals/0012-add-noescape-to-public-library-api.md>, which is also about @noescape, but that proposal is about modifying C/Obj-C API, so it’s not a good fit <https://github.com/apple/swift-evolution/pull/122>.

Introduction

stdlib’s ManagedBuffer family of APIs has some public initializers and methods taking closures that are missing the @noescape attribute. The same family has a set of withUnsafeMutablePointer* functions that do not have a rethrows declaration, while all similar methods elsewhere in the standard library allow closures to throw.

I propose to add the missing @noescape attributes and rethrows.

Motivation

- ManagedBuffer seems designed for raw performance. Not having @noescape on the only API that allows access to the buffer’s contents defeats some compiler optimizations, such as omitting unneccessary retain/release calls. This can negate the performance advantage of ManagedBuffer over simpler solutions, like using an Array.
- Accepting throwing closures makes these APIs more versatile, and also improves their consistency with other parts of stdlib.

Detailed Design

The following set of APIs would be affected by this proposal:

public class ManagedProtoBuffer<Value, Element> : NonObjectiveCBase {
  public final func withUnsafeMutablePointerToValue<R>(body: (UnsafeMutablePointer<Value>) -> R) -> R
  public final func withUnsafeMutablePointerToElements<R>(body: (UnsafeMutablePointer<Element>) -> R) -> R
  public final func withUnsafeMutablePointers<R>(body: (_: UnsafeMutablePointer<Value>, _: UnsafeMutablePointer<Element>) -> R) -> R
}

public class ManagedBuffer<Value, Element> : ManagedProtoBuffer<Value, Element> {
  public final class func create(minimumCapacity: Int, initialValue: (ManagedProtoBuffer<Value,Element>) -> Value) -> ManagedBuffer<Value,Element>
}

public struct ManagedBufferPointer<Value, Element> : Equatable {
  public init(bufferClass: AnyClass, minimumCapacity: Int, initialValue: (buffer: AnyObject, allocatedCount: (AnyObject) -> Int) -> Value)
  public func withUnsafeMutablePointerToValue<R>(body: (UnsafeMutablePointer<Value>) -> R) -> R
  public func withUnsafeMutablePointerToElements<R>(body: (UnsafeMutablePointer<Element>) -> R) -> R
  public func withUnsafeMutablePointers<R>(body: (_: UnsafeMutablePointer<Value>, _: UnsafeMutablePointer<Element>) -> R) -> R
}

Here is how they would look after the proposed changes:

public class ManagedProtoBuffer<Value, Element> : NonObjectiveCBase {
  public final func withUnsafeMutablePointerToValue<R>(@noescape body: (UnsafeMutablePointer<Value>) throws -> R) rethrows -> R
  public final func withUnsafeMutablePointerToElements<R>(@noescape body: (UnsafeMutablePointer<Element>) throws -> R) rethrows -> R
  public final func withUnsafeMutablePointers<R>(@noescape body: (_: UnsafeMutablePointer<Value>, _: UnsafeMutablePointer<Element>) throws -> R) rethrows -> R

public class ManagedBuffer<Value, Element> : ManagedProtoBuffer<Value, Element> {
  public final class func create(minimumCapacity: Int, @noescape initialValue: (ManagedProtoBuffer<Value,Element>) -> Value) -> ManagedBuffer<Value,Element>
}

public struct ManagedBufferPointer<Value, Element> : Equatable {
  public init(bufferClass: AnyClass, minimumCapacity: Int, @noescape initialValue: (buffer: AnyObject, allocatedCount: (AnyObject) -> Int) -> Value)
  public func withUnsafeMutablePointerToValue<R>(@noescape body: (UnsafeMutablePointer<Value>) throws -> R) rethrows -> R
  public func withUnsafeMutablePointerToElements<R>(@noescape body: (UnsafeMutablePointer<Element>) throws -> R) rethrows -> R
  public func withUnsafeMutablePointers<R>(@noescape body: (_: UnsafeMutablePointer<Value>,_: UnsafeMutablePointer<Element>) throws -> R) rethrows -> R
}

A draft implementation is available at https://github.com/apple/swift/compare/master...lorentey:noescape

Impact on Existing Code

Luckily, all modified API is either marked final, or defined in a struct, so I expect no existing code is going to break due to these changes.

···

--
Károly


(Dave Abrahams) #2

I’d like to request feedback on the draft proposal below, about a (hopefully) trivial change to the standard library.

I considered adding it to SE-0012
<https://github.com/apple/swift-evolution/blob/master/proposals/0012-add-noescape-to-public-library-api.md>,
which is also about @noescape, but that proposal is about modifying
C/Obj-C API, so it’s not a good fit
<https://github.com/apple/swift-evolution/pull/122>.

Introduction

stdlib’s ManagedBuffer family of APIs has some public initializers and
methods taking closures that are missing the @noescape attribute. The
same family has a set of withUnsafeMutablePointer* functions that do
not have a rethrows declaration, while all similar methods elsewhere
in the standard library allow closures to throw.

I propose to add the missing @noescape attributes and rethrows.

On first look this seems to be a great idea. Have you checked for
performance impact?

···

on Sat Feb 06 2016, Károly Lőrentey <swift-evolution@swift.org> wrote:

Motivation

- ManagedBuffer seems designed for raw performance. Not having
@noescape on the only API that allows access to the buffer’s contents
defeats some compiler optimizations, such as omitting unneccessary
retain/release calls. This can negate the performance advantage of
ManagedBuffer over simpler solutions, like using an Array.
- Accepting throwing closures makes these APIs more versatile, and
also improves their consistency with other parts of stdlib.

Detailed Design

The following set of APIs would be affected by this proposal:

public class ManagedProtoBuffer<Value, Element> : NonObjectiveCBase {
  public final func withUnsafeMutablePointerToValue<R>(body: (UnsafeMutablePointer<Value>) -> R) -> R
  public final func withUnsafeMutablePointerToElements<R>(body: (UnsafeMutablePointer<Element>) -> R) -> R
  public final func withUnsafeMutablePointers<R>(body: (_:
UnsafeMutablePointer<Value>, _: UnsafeMutablePointer<Element>) -> R)
-> R
}

public class ManagedBuffer<Value, Element> : ManagedProtoBuffer<Value, Element> {
  public final class func create(minimumCapacity: Int, initialValue:
(ManagedProtoBuffer<Value,Element>) -> Value) ->
ManagedBuffer<Value,Element>
}

public struct ManagedBufferPointer<Value, Element> : Equatable {
  public init(bufferClass: AnyClass, minimumCapacity: Int,
initialValue: (buffer: AnyObject, allocatedCount: (AnyObject) -> Int)
-> Value)
  public func withUnsafeMutablePointerToValue<R>(body: (UnsafeMutablePointer<Value>) -> R) -> R
  public func withUnsafeMutablePointerToElements<R>(body: (UnsafeMutablePointer<Element>) -> R) -> R
  public func withUnsafeMutablePointers<R>(body: (_:
UnsafeMutablePointer<Value>, _: UnsafeMutablePointer<Element>) -> R)
-> R
}

Here is how they would look after the proposed changes:

public class ManagedProtoBuffer<Value, Element> : NonObjectiveCBase {
  public final func withUnsafeMutablePointerToValue<R>(@noescape body:
(UnsafeMutablePointer<Value>) throws -> R) rethrows -> R
  public final func withUnsafeMutablePointerToElements<R>(@noescape
body: (UnsafeMutablePointer<Element>) throws -> R) rethrows -> R
  public final func withUnsafeMutablePointers<R>(@noescape body: (_:
UnsafeMutablePointer<Value>, _: UnsafeMutablePointer<Element>) throws
-> R) rethrows -> R

public class ManagedBuffer<Value, Element> : ManagedProtoBuffer<Value, Element> {
  public final class func create(minimumCapacity: Int, @noescape
initialValue: (ManagedProtoBuffer<Value,Element>) -> Value) ->
ManagedBuffer<Value,Element>
}

public struct ManagedBufferPointer<Value, Element> : Equatable {
  public init(bufferClass: AnyClass, minimumCapacity: Int, @noescape
initialValue: (buffer: AnyObject, allocatedCount: (AnyObject) -> Int)
-> Value)
  public func withUnsafeMutablePointerToValue<R>(@noescape body:
(UnsafeMutablePointer<Value>) throws -> R) rethrows -> R
  public func withUnsafeMutablePointerToElements<R>(@noescape body:
(UnsafeMutablePointer<Element>) throws -> R) rethrows -> R
  public func withUnsafeMutablePointers<R>(@noescape body: (_:
UnsafeMutablePointer<Value>,_: UnsafeMutablePointer<Element>) throws
-> R) rethrows -> R
}

A draft implementation is available at
https://github.com/apple/swift/compare/master...lorentey:noescape
<https://github.com/apple/swift/compare/master...lorentey:noescape>

Impact on Existing Code

Luckily, all modified API is either marked final, or defined in a
struct, so I expect no existing code is going to break due to these
changes.

--
-Dave


(Janosch Hildebrand) #3

+1

Is there a reason for not marking 'ManagedBuffer.create' and 'ManagedBufferPointer.init' as rethrows and taking throwing closures?
Now that we can have throwing initializers I don't really see a reason for not allowing throwing closures here...

···

On 06 Feb 2016, at 21:03, Károly Lőrentey via swift-evolution <swift-evolution@swift.org> wrote:

I’d like to request feedback on the draft proposal below, about a (hopefully) trivial change to the standard library.

I considered adding it to SE-0012 <https://github.com/apple/swift-evolution/blob/master/proposals/0012-add-noescape-to-public-library-api.md>, which is also about @noescape, but that proposal is about modifying C/Obj-C API, so it’s not a good fit <https://github.com/apple/swift-evolution/pull/122>.

Introduction

stdlib’s ManagedBuffer family of APIs has some public initializers and methods taking closures that are missing the @noescape attribute. The same family has a set of withUnsafeMutablePointer* functions that do not have a rethrows declaration, while all similar methods elsewhere in the standard library allow closures to throw.

I propose to add the missing @noescape attributes and rethrows.

Motivation

- ManagedBuffer seems designed for raw performance. Not having @noescape on the only API that allows access to the buffer’s contents defeats some compiler optimizations, such as omitting unneccessary retain/release calls. This can negate the performance advantage of ManagedBuffer over simpler solutions, like using an Array.
- Accepting throwing closures makes these APIs more versatile, and also improves their consistency with other parts of stdlib.

Detailed Design

The following set of APIs would be affected by this proposal:

public class ManagedProtoBuffer<Value, Element> : NonObjectiveCBase {
  public final func withUnsafeMutablePointerToValue<R>(body: (UnsafeMutablePointer<Value>) -> R) -> R
  public final func withUnsafeMutablePointerToElements<R>(body: (UnsafeMutablePointer<Element>) -> R) -> R
  public final func withUnsafeMutablePointers<R>(body: (_: UnsafeMutablePointer<Value>, _: UnsafeMutablePointer<Element>) -> R) -> R
}

public class ManagedBuffer<Value, Element> : ManagedProtoBuffer<Value, Element> {
  public final class func create(minimumCapacity: Int, initialValue: (ManagedProtoBuffer<Value,Element>) -> Value) -> ManagedBuffer<Value,Element>
}

public struct ManagedBufferPointer<Value, Element> : Equatable {
  public init(bufferClass: AnyClass, minimumCapacity: Int, initialValue: (buffer: AnyObject, allocatedCount: (AnyObject) -> Int) -> Value)
  public func withUnsafeMutablePointerToValue<R>(body: (UnsafeMutablePointer<Value>) -> R) -> R
  public func withUnsafeMutablePointerToElements<R>(body: (UnsafeMutablePointer<Element>) -> R) -> R
  public func withUnsafeMutablePointers<R>(body: (_: UnsafeMutablePointer<Value>, _: UnsafeMutablePointer<Element>) -> R) -> R
}

Here is how they would look after the proposed changes:

public class ManagedProtoBuffer<Value, Element> : NonObjectiveCBase {
  public final func withUnsafeMutablePointerToValue<R>(@noescape body: (UnsafeMutablePointer<Value>) throws -> R) rethrows -> R
  public final func withUnsafeMutablePointerToElements<R>(@noescape body: (UnsafeMutablePointer<Element>) throws -> R) rethrows -> R
  public final func withUnsafeMutablePointers<R>(@noescape body: (_: UnsafeMutablePointer<Value>, _: UnsafeMutablePointer<Element>) throws -> R) rethrows -> R

public class ManagedBuffer<Value, Element> : ManagedProtoBuffer<Value, Element> {
  public final class func create(minimumCapacity: Int, @noescape initialValue: (ManagedProtoBuffer<Value,Element>) -> Value) -> ManagedBuffer<Value,Element>
}

public struct ManagedBufferPointer<Value, Element> : Equatable {
  public init(bufferClass: AnyClass, minimumCapacity: Int, @noescape initialValue: (buffer: AnyObject, allocatedCount: (AnyObject) -> Int) -> Value)
  public func withUnsafeMutablePointerToValue<R>(@noescape body: (UnsafeMutablePointer<Value>) throws -> R) rethrows -> R
  public func withUnsafeMutablePointerToElements<R>(@noescape body: (UnsafeMutablePointer<Element>) throws -> R) rethrows -> R
  public func withUnsafeMutablePointers<R>(@noescape body: (_: UnsafeMutablePointer<Value>,_: UnsafeMutablePointer<Element>) throws -> R) rethrows -> R
}

A draft implementation is available at https://github.com/apple/swift/compare/master...lorentey:noescape

Impact on Existing Code

Luckily, all modified API is either marked final, or defined in a struct, so I expect no existing code is going to break due to these changes.

--
Károly

_______________________________________________
swift-evolution mailing list
swift-evolution@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution

- Janosch


(Jacob Bandes-Storch) #4

As the author of SE-0012, +1 :slight_smile:

···

On Sun, Feb 7, 2016 at 7:50 AM Dave Abrahams via swift-evolution < swift-evolution@swift.org> wrote:

on Sat Feb 06 2016, Károly Lőrentey <swift-evolution@swift.org> wrote:

> I’d like to request feedback on the draft proposal below, about a
(hopefully) trivial change to the standard library.
>
> I considered adding it to SE-0012
> <
https://github.com/apple/swift-evolution/blob/master/proposals/0012-add-noescape-to-public-library-api.md
>,
> which is also about @noescape, but that proposal is about modifying
> C/Obj-C API, so it’s not a good fit
> <https://github.com/apple/swift-evolution/pull/122>.
>
> Introduction
>
> stdlib’s ManagedBuffer family of APIs has some public initializers and
> methods taking closures that are missing the @noescape attribute. The
> same family has a set of withUnsafeMutablePointer* functions that do
> not have a rethrows declaration, while all similar methods elsewhere
> in the standard library allow closures to throw.
>
> I propose to add the missing @noescape attributes and rethrows.

On first look this seems to be a great idea. Have you checked for
performance impact?

> Motivation
>
> - ManagedBuffer seems designed for raw performance. Not having
> @noescape on the only API that allows access to the buffer’s contents
> defeats some compiler optimizations, such as omitting unneccessary
> retain/release calls. This can negate the performance advantage of
> ManagedBuffer over simpler solutions, like using an Array.
> - Accepting throwing closures makes these APIs more versatile, and
> also improves their consistency with other parts of stdlib.
>
> Detailed Design
>
> The following set of APIs would be affected by this proposal:
>
> public class ManagedProtoBuffer<Value, Element> : NonObjectiveCBase {
> public final func withUnsafeMutablePointerToValue<R>(body:
(UnsafeMutablePointer<Value>) -> R) -> R
> public final func withUnsafeMutablePointerToElements<R>(body:
(UnsafeMutablePointer<Element>) -> R) -> R
> public final func withUnsafeMutablePointers<R>(body: (_:
> UnsafeMutablePointer<Value>, _: UnsafeMutablePointer<Element>) -> R)
> -> R
> }
>
> public class ManagedBuffer<Value, Element> : ManagedProtoBuffer<Value,
> {
> public final class func create(minimumCapacity: Int, initialValue:
> (ManagedProtoBuffer<Value,Element>) -> Value) ->
> ManagedBuffer<Value,Element>
> }
>
> public struct ManagedBufferPointer<Value, Element> : Equatable {
> public init(bufferClass: AnyClass, minimumCapacity: Int,
> initialValue: (buffer: AnyObject, allocatedCount: (AnyObject) -> Int)
> -> Value)
> public func withUnsafeMutablePointerToValue<R>(body:
(UnsafeMutablePointer<Value>) -> R) -> R
> public func withUnsafeMutablePointerToElements<R>(body:
(UnsafeMutablePointer<Element>) -> R) -> R
> public func withUnsafeMutablePointers<R>(body: (_:
> UnsafeMutablePointer<Value>, _: UnsafeMutablePointer<Element>) -> R)
> -> R
> }
>
> Here is how they would look after the proposed changes:
>
> public class ManagedProtoBuffer<Value, Element> : NonObjectiveCBase {
> public final func withUnsafeMutablePointerToValue<R>(@noescape body:
> (UnsafeMutablePointer<Value>) throws -> R) rethrows -> R
> public final func withUnsafeMutablePointerToElements<R>(@noescape
> body: (UnsafeMutablePointer<Element>) throws -> R) rethrows -> R
> public final func withUnsafeMutablePointers<R>(@noescape body: (_:
> UnsafeMutablePointer<Value>, _: UnsafeMutablePointer<Element>) throws
> -> R) rethrows -> R
>
> public class ManagedBuffer<Value, Element> : ManagedProtoBuffer<Value,
> {
> public final class func create(minimumCapacity: Int, @noescape
> initialValue: (ManagedProtoBuffer<Value,Element>) -> Value) ->
> ManagedBuffer<Value,Element>
> }
>
> public struct ManagedBufferPointer<Value, Element> : Equatable {
> public init(bufferClass: AnyClass, minimumCapacity: Int, @noescape
> initialValue: (buffer: AnyObject, allocatedCount: (AnyObject) -> Int)
> -> Value)
> public func withUnsafeMutablePointerToValue<R>(@noescape body:
> (UnsafeMutablePointer<Value>) throws -> R) rethrows -> R
> public func withUnsafeMutablePointerToElements<R>(@noescape body:
> (UnsafeMutablePointer<Element>) throws -> R) rethrows -> R
> public func withUnsafeMutablePointers<R>(@noescape body: (_:
> UnsafeMutablePointer<Value>,_: UnsafeMutablePointer<Element>) throws
> -> R) rethrows -> R
> }
>
> A draft implementation is available at
> https://github.com/apple/swift/compare/master...lorentey:noescape
> <https://github.com/apple/swift/compare/master...lorentey:noescape>
>
> Impact on Existing Code
>
> Luckily, all modified API is either marked final, or defined in a
> struct, so I expect no existing code is going to break due to these
> changes.

--
-Dave

_______________________________________________
swift-evolution mailing list
swift-evolution@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution


(Karoy Lorentey) #5

Yes, although I found it challenging to create a good
microbenchmark for this. Subtle changes in the benchmarking code
lead to large swings in runtime performance, which makes me question
the usefulness of my results.

Keeping that in mind, for a trivial ManagedBuffer subclass, I found
that @noescape makes for a ~15-18% improvement when whole module
optimization is disabled, or when the subclass is imported.

Throwing in the rethrows declarations reduces the improvement to
~9-13%, or (in the case of a particular subscript getter test) even
reverses it, making the code ~3% slower.

The proposal has no discernible impact on ManagedBuffer subclasses
that the optimizer has full access to. (I.e., when they’re defined
in the same file as the code that’s using them, or in the same
module with WMO.) Unoptimized code also seems unaffected by these
changes.

My benchmarking code is on GitHub; feedback would be very welcome:

    https://github.com/lorentey/ManagedBuffer-Benchmark

···

On 2016-02-07, at 16:18, Dave Abrahams via swift-evolution <swift-evolution@swift.org> wrote:

On first look this seems to be a great idea. Have you checked for
performance impact?

--
Karoly