Open Issues Affecting Standard Library API Stability


(Dmitri Gribenko) #1

Hi swift-evolution,

Dave, Max and I have compiled a list of open issues in the standard
library for which the resolutions could result non-additive API
changes. Having a resolution (and an implementation of the
resolution!) for these issues is blocking API stability.

https://gist.github.com/gribozavr/37e811f12b27c6365fc88e6f9645634d

This is not an exhaustive list. The first two sections ("Low-Hanging
Fruit" and "Moderate") list issues with low and moderate difficulty.
It should be possible to resolve them with minimal design work.

**We are strongly encouraging the community to pick up these issues.
Please do not reply to this thread. Instead, start a new thread for
the issue you would like to discuss, and include the original
description and the bugs.swift.org link in your email.**

For the issues in the "Hard" section, we would like to post the
following commentary:

- The issue about algorithms not marked mutating consuming a
single-pass Sequences. We have figured out a design that we think is
a good trade-off. We will post it for public discussion within a few
days.

- An extension to the collections subsystem that would allow us to
model infinite data streams. We think that the answer to this problem
should try hard to handle both single- and multi-pass collections, or
provide a very convincing explanation why it is desired to enable only
one of them (only multi-pass or only single-pass) to be infinite.

We see four possible solutions to this problem:

1. No change (disallow infinite data streams).

2. Allow infinite single-pass streams (as Iterators) and infinite
multi-pass streams (as Collections).

3. #2, and, for QoI, to catch mistakes in a non-guaranteed,
best-effort fashion, add a property 'isKnownToBeInfinite' or
'isKnownToBeFinite' that would be checked before the whole dataset is
iterated. If the dataset is known to be infinite, issue a trap.

4. #2, and introduce new protocols for finite iterators and finite collections.

Unfortunately we don't have time to investigate the question about
infinite collections further. We suggest interested community members
to drive the discussion on this topic.

- Dave Abrahams has been investigating whether it would be an
improvement to remove Comparable requirement from collection indices,
he will post an update separately.

- Naming of algorithms on sequences and collections is important, and
we are thankful for the community for working on these issues. We
hope that these efforts would result in proposals!

- The spaceship <=> operator is an important, but also complex topic.
If someone wants to work on this item, we think that writing a
prototype and showing how it integrates with existing code,
investigating whether it solves issues with floating point min() and
NaNs, figuring out what the migration experience is etc. is critical
before opening a discussion.

The general design direction is clear -- we want to require users to
define just one operator, <=>, to conform to Comparable, but there are
many details that would only be found in a prototype implementation.
These small issues need concrete answers.

I'd like to thank the community again for all work that you are doing,
and encourage to pick up the issues from the list, and start
discussions in new threads. I'd like to emphasize low and moderate
difficulty items. We believe that for these issues the design space
is small, and consensus can be reached without a lot of discussion.
The associated API changes have a good chance to land in Swift 3.x.

Dmitri

···

--
main(i,j){for(i=2;;i++){for(j=2;j<i;j++){if(!(i%j)){j=0;break;}}if
(j){printf("%d\n",i);}}} /*Dmitri Gribenko <gribozavr@gmail.com>*/


(Chris Lattner) #2

Thank you for collecting this Dmitri! For the issues in the “low hanging fruit” list, are the changes all sufficiently "obvious”? If so, having one proposal tackle all of them in one sweep would be preferable to reduce process overhead.

-Chris

···

On Jul 5, 2016, at 6:57 PM, Dmitri Gribenko via swift-evolution <swift-evolution@swift.org> wrote:

Hi swift-evolution,

Dave, Max and I have compiled a list of open issues in the standard
library for which the resolutions could result non-additive API
changes. Having a resolution (and an implementation of the
resolution!) for these issues is blocking API stability.

https://gist.github.com/gribozavr/37e811f12b27c6365fc88e6f9645634d


(Dmitri Gribenko) #3

Hi swift-evolution,

Dave, Max and I have compiled a list of open issues in the standard
library for which the resolutions could result non-additive API
changes. Having a resolution (and an implementation of the
resolution!) for these issues is blocking API stability.

https://gist.github.com/gribozavr/37e811f12b27c6365fc88e6f9645634d

Thank you for collecting this Dmitri! For the issues in the “low hanging fruit” list, are the changes all sufficiently "obvious”? If so, having one proposal tackle all of them in one sweep would be preferable to reduce process overhead.

My subjective assessment:

The global function withUnsafe[Mutable]Pointer(&x) should have an argument label “to”.

Obvious.

UnicodeScalar.init(Int) should be failable.

Obvious.

withUnsafePointer shouldn't take its argument as inout.

Jordan has objections, see https://bugs.swift.org/browse/SR-1956

Remove unsafeAddressOf. We are not aware of any real usecases for it. If there are any, it should be renamed to unsafeAddress(of:) to follow the guidelines.

Obvious, unless someone comes up with use cases during the review period.

Consider renaming or eliminating ManagedProtoBuffer.
The reason why ManagedProtoBuffer exists is to give the users an extra bit of type safety inside of the closure passed to ManagedBuffer.create().

Debatable.

String.CharacterView.Iterator should be a custom type rather than the default, to allow performance optimizations. Audit all other collections for such opportunities.

Obvious.

String(count:, repeatedValue:) and String.append() are ambiguous without an explicit cast to Character.

Obvious.

Rename the ~LiteralConvertible protocols according to the resolution of the proposal currently under review.

A separate review.

Dmitri

···

On Tue, Jul 5, 2016 at 9:24 PM, Chris Lattner <clattner@apple.com> wrote:

On Jul 5, 2016, at 6:57 PM, Dmitri Gribenko via swift-evolution <swift-evolution@swift.org> wrote:

--
main(i,j){for(i=2;;i++){for(j=2;j<i;j++){if(!(i%j)){j=0;break;}}if
(j){printf("%d\n",i);}}} /*Dmitri Gribenko <gribozavr@gmail.com>*/


(Erica Sadun) #4

Hi swift-evolution,

Dave, Max and I have compiled a list of open issues in the standard
library for which the resolutions could result non-additive API
changes. Having a resolution (and an implementation of the
resolution!) for these issues is blocking API stability.

https://gist.github.com/gribozavr/37e811f12b27c6365fc88e6f9645634d

Thank you for collecting this Dmitri! For the issues in the “low hanging fruit” list, are the changes all sufficiently "obvious”? If so, having one proposal tackle all of them in one sweep would be preferable to reduce process overhead.

My subjective assessment:

The global function withUnsafe[Mutable]Pointer(&x) should have an argument label “to”.

Obvious.

Remove unsafeAddressOf. We are not aware of any real usecases for it. If there are any, it should be renamed to unsafeAddress(of:) to follow the guidelines.

Obvious, unless someone comes up with use cases during the review period.

Consider renaming or eliminating ManagedProtoBuffer.
The reason why ManagedProtoBuffer exists is to give the users an extra bit of type safety inside of the closure passed to ManagedBuffer.create().

Debatable.

withUnsafePointer shouldn't take its argument as inout.

Jordan has objections, see https://bugs.swift.org/browse/SR-1956

These batch together but I'm thinking the latter three will have much more debate. (The first one can be a quick hit.) I'll get these topics started in a separate thread.

UnicodeScalar.init(Int) should be failable.

Obvious.

String.CharacterView.Iterator should be a custom type rather than the default, to allow performance optimizations. Audit all other collections for such opportunities.

Obvious.

String(count:, repeatedValue:) and String.append() are ambiguous without an explicit cast to Character.

Obvious.

These string items naturally batch together

Rename the ~LiteralConvertible protocols according to the resolution of the proposal currently under review.

A separate review.

I think Matthew Johnson has this under control

···

On Jul 5, 2016, at 11:50 PM, Dmitri Gribenko via swift-evolution <swift-evolution@swift.org> wrote:
On Tue, Jul 5, 2016 at 9:24 PM, Chris Lattner <clattner@apple.com <mailto:clattner@apple.com>> wrote:

On Jul 5, 2016, at 6:57 PM, Dmitri Gribenko via swift-evolution <swift-evolution@swift.org> wrote:


(Jacob Bandes-Storch) #5

It's minor, but I use unsafeAddressOf regularly for writing `description`
methods:

    var description: String {
        return "<\(self.dynamicType): \(unsafeAddressOf(self))>{ more info
here... }"
    }

I guess this would be covered by some generalized solution for format
specifiers in string interpolations, but I gather that won't happen for
quite a while...

···

On Tue, Jul 5, 2016 at 10:50 PM, Dmitri Gribenko via swift-evolution < swift-evolution@swift.org> wrote:

> Remove unsafeAddressOf. We are not aware of any real usecases for it. If
there are any, it should be renamed to unsafeAddress(of:) to follow the
guidelines.
Obvious, unless someone comes up with use cases during the review period.


(Karl) #6

I’m quite interested in this point:

Figure out whether RangeReplaceableCollection.replaceRange should accept a Sequence as its argument, and if so implement it.

At the moment `replaceSubrange` takes a sequence, and lots of methods on RangeReplaceableCollection use that to perform their mutation operations. That means that Collections gets iterated item-by-item; for example, a String will get parsed out in to Characters and added individually to the existing string. In general, depending on which methods you use to mutate a RRC, you can expect wildly different performance characteristics. So it’s more important to fix than the name might suggest.

I had a PR open for this which added a Collection specialisation, but you said this could be handled in a more general way which allows for more optimised mutations. I’m curious how this would work; how does `RangeReplaceableCollection.replaceSubrange<S:Sequence>(range:, with: S)` call a more optimised implementation if S also conforms to Collection, if not by adding a specialisation?

Karl

···

On 6 Jul 2016, at 07:50, Dmitri Gribenko via swift-evolution <swift-evolution@swift.org> wrote:

On Tue, Jul 5, 2016 at 9:24 PM, Chris Lattner <clattner@apple.com> wrote:

On Jul 5, 2016, at 6:57 PM, Dmitri Gribenko via swift-evolution <swift-evolution@swift.org> wrote:

Hi swift-evolution,

Dave, Max and I have compiled a list of open issues in the standard
library for which the resolutions could result non-additive API
changes. Having a resolution (and an implementation of the
resolution!) for these issues is blocking API stability.

https://gist.github.com/gribozavr/37e811f12b27c6365fc88e6f9645634d

Thank you for collecting this Dmitri! For the issues in the “low hanging fruit” list, are the changes all sufficiently "obvious”? If so, having one proposal tackle all of them in one sweep would be preferable to reduce process overhead.

My subjective assessment:

The global function withUnsafe[Mutable]Pointer(&x) should have an argument label “to”.

Obvious.

UnicodeScalar.init(Int) should be failable.

Obvious.

withUnsafePointer shouldn't take its argument as inout.

Jordan has objections, see https://bugs.swift.org/browse/SR-1956

Remove unsafeAddressOf. We are not aware of any real usecases for it. If there are any, it should be renamed to unsafeAddress(of:) to follow the guidelines.

Obvious, unless someone comes up with use cases during the review period.

Consider renaming or eliminating ManagedProtoBuffer.
The reason why ManagedProtoBuffer exists is to give the users an extra bit of type safety inside of the closure passed to ManagedBuffer.create().

Debatable.

String.CharacterView.Iterator should be a custom type rather than the default, to allow performance optimizations. Audit all other collections for such opportunities.

Obvious.

String(count:, repeatedValue:) and String.append() are ambiguous without an explicit cast to Character.

Obvious.

Rename the ~LiteralConvertible protocols according to the resolution of the proposal currently under review.

A separate review.

Dmitri

--
main(i,j){for(i=2;;i++){for(j=2;j<i;j++){if(!(i%j)){j=0;break;}}if
(j){printf("%d\n",i);}}} /*Dmitri Gribenko <gribozavr@gmail.com>*/
_______________________________________________
swift-evolution mailing list
swift-evolution@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution


(Dave Abrahams) #7

Hi swift-evolution,

Dave, Max and I have compiled a list of open issues in the standard
library for which the resolutions could result non-additive API
changes. Having a resolution (and an implementation of the
resolution!) for these issues is blocking API stability.

https://gist.github.com/gribozavr/37e811f12b27c6365fc88e6f9645634d

Thank you for collecting this Dmitri! For the issues in the “low
hanging fruit” list, are the changes all sufficiently "obvious”? If
so, having one proposal tackle all of them in one sweep would be
preferable to reduce process overhead.

My subjective assessment:

The global function withUnsafe[Mutable]Pointer(&x) should have an argument label “to”.

Obvious.

UnicodeScalar.init(Int) should be failable.

Obvious.

withUnsafePointer shouldn't take its argument as inout.

Jordan has objections, see https://bugs.swift.org/browse/SR-1956

Remove unsafeAddressOf. We are not aware of any real usecases for
it. If there are any, it should be renamed to unsafeAddress(of:) to
follow the guidelines.

Obvious, unless someone comes up with use cases during the review period.

Consider renaming or eliminating ManagedProtoBuffer.
The reason why ManagedProtoBuffer exists is to give the users an
extra bit of type safety inside of the closure passed to
ManagedBuffer.create().

Debatable.

The only debate, IMO, is whether to rename or eliminate. Anything with
“ProtoBuf” in the name is likely to be misleading.

···

on Tue Jul 05 2016, Dmitri Gribenko <swift-evolution@swift.org> wrote:

On Tue, Jul 5, 2016 at 9:24 PM, Chris Lattner <clattner@apple.com> wrote:

On Jul 5, 2016, at 6:57 PM, Dmitri Gribenko via swift-evolution <swift-evolution@swift.org> wrote:

String.CharacterView.Iterator should be a custom type rather than
the default, to allow performance optimizations. Audit all other
collections for such opportunities.

Obvious.

String(count:, repeatedValue:) and String.append() are ambiguous without an explicit cast to Character.

Obvious.

Rename the ~LiteralConvertible protocols according to the resolution of the proposal currently under review.

A separate review.

Dmitri

--
Dave


(Harlan Haskins) #8

I’ve also seen unsafeAddressOf(_:slight_smile: used when interfacing with C function pointers when the lifetime of an object is guaranteed. Many C APIs vend an API like:

void perform_action(void (*callback)(void *data), void *initial_data);

For which it is expected to use unsafeAddressOf on a class instance, like:

perform_action({ data in
  let _self = unsafeBitCast(data, to: MyClass.self)
  _self.foo()
}, data: unsafeAddressOf(self))

It’s unsafe and error-prone, sure, but that’s why we have `unsafe` in the name :sweat_smile: — I’ve had to use this to interface with libclang.

(For an existing example of this, see SwiftGtk: https://github.com/TomasLinhart/SwiftGtk/blob/master/Sources/Application.swift)

— Harlan

···

On Jul 6, 2016, at 11:01 AM, Jacob Bandes-Storch via swift-evolution <swift-evolution@swift.org> wrote:

On Tue, Jul 5, 2016 at 10:50 PM, Dmitri Gribenko via swift-evolution <swift-evolution@swift.org <mailto:swift-evolution@swift.org>> wrote:

> Remove unsafeAddressOf. We are not aware of any real usecases for it. If there are any, it should be renamed to unsafeAddress(of:) to follow the guidelines.
Obvious, unless someone comes up with use cases during the review period.

It's minor, but I use unsafeAddressOf regularly for writing `description` methods:

    var description: String {
        return "<\(self.dynamicType): \(unsafeAddressOf(self))>{ more info here... }"
    }

I guess this would be covered by some generalized solution for format specifiers in string interpolations, but I gather that won't happen for quite a while...
_______________________________________________
swift-evolution mailing list
swift-evolution@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution


(Dave Abrahams) #9

I’m quite interested in this point:

Figure out whether RangeReplaceableCollection.replaceRange should accept a Sequence as its argument, and if so implement it.

At the moment `replaceSubrange` takes a sequence,

No, actually at the moment it takes a Collection.

and lots of methods on RangeReplaceableCollection use that to perform
their mutation operations. That means that Collections gets iterated
item-by-item; for example, a String will get parsed out in to
Characters and added individually to the existing string.

And a String is neither a Sequence nor a Collection.

In general, depending on which methods you use to mutate a RRC, you
can expect wildly different performance characteristics. So it’s more
important to fix than the name might suggest.

It depends on the RRC. Some RRCs can be restructured at the front or
back more cheaply than in the middle.

I had a PR open for this which added a Collection specialisation, but
you said this could be handled in a more general way which allows for
more optimised mutations. I’m curious how this would work; how does
`RangeReplaceableCollection.replaceSubrange<S:Sequence>(range:, with:
S)` call a more optimised implementation if S also conforms to
Collection, if not by adding a specialisation?

I'm sorry, I don't know what you're referring to here. If you have
specific questions about a response to a PR, could you ask them in the
PR? (It doesn't have to be open)

···

on Wed Jul 06 2016, Karl <razielim-AT-gmail.com> wrote:

On 6 Jul 2016, at 07:50, Dmitri Gribenko via swift-evolution <swift-evolution@swift.org> wrote:

On Tue, Jul 5, 2016 at 9:24 PM, Chris Lattner <clattner@apple.com> wrote:

On Jul 5, 2016, at 6:57 PM, Dmitri Gribenko via swift-evolution <swift-evolution@swift.org> wrote:

Hi swift-evolution,

Dave, Max and I have compiled a list of open issues in the standard
library for which the resolutions could result non-additive API
changes. Having a resolution (and an implementation of the
resolution!) for these issues is blocking API stability.

https://gist.github.com/gribozavr/37e811f12b27c6365fc88e6f9645634d

Thank you for collecting this Dmitri! For the issues in the “low
hanging fruit” list, are the changes all sufficiently "obvious”?
If so, having one proposal tackle all of them in one sweep would be
preferable to reduce process overhead.

My subjective assessment:

The global function withUnsafe[Mutable]Pointer(&x) should have an argument label “to”.

Obvious.

UnicodeScalar.init(Int) should be failable.

Obvious.

withUnsafePointer shouldn't take its argument as inout.

Jordan has objections, see https://bugs.swift.org/browse/SR-1956

Remove unsafeAddressOf. We are not aware of any real usecases for
it. If there are any, it should be renamed to unsafeAddress(of:) to
follow the guidelines.

Obvious, unless someone comes up with use cases during the review period.

Consider renaming or eliminating ManagedProtoBuffer.
The reason why ManagedProtoBuffer exists is to give the users an
extra bit of type safety inside of the closure passed to
ManagedBuffer.create().

Debatable.

String.CharacterView.Iterator should be a custom type rather than
the default, to allow performance optimizations. Audit all other
collections for such opportunities.

Obvious.

String(count:, repeatedValue:) and String.append() are ambiguous without an explicit cast to Character.

Obvious.

Rename the ~LiteralConvertible protocols according to the resolution of the proposal currently under review.

A separate review.

Dmitri

--
main(i,j){for(i=2;;i++){for(j=2;j<i;j++){if(!(i%j)){j=0;break;}}if
(j){printf("%d\n",i);}}} /*Dmitri Gribenko <gribozavr@gmail.com>*/
_______________________________________________
swift-evolution mailing list
swift-evolution@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution

--
Dave


(Dave Abrahams) #10

> Remove unsafeAddressOf. We are not aware of any real usecases for it. If
there are any, it should be renamed to unsafeAddress(of:) to follow the
guidelines.
Obvious, unless someone comes up with use cases during the review period.

It's minor, but I use unsafeAddressOf regularly for writing `description`
methods:

    var description: String {
        return "<\(self.dynamicType): \(unsafeAddressOf(self))>{ more info
here... }"
    }

You can do that, but there's no guarantee that the address is
meaningful. The compiler is free to copy self into a writeback buffer
whose address gets printed.

···

on Wed Jul 06 2016, Jacob Bandes-Storch <jtbandes-AT-gmail.com> wrote:

On Tue, Jul 5, 2016 at 10:50 PM, Dmitri Gribenko via swift-evolution < > swift-evolution@swift.org> wrote:

I guess this would be covered by some generalized solution for format
specifiers in string interpolations, but I gather that won't happen for
quite a while...

--
Dave


(Dmitri Gribenko) #11

The RRC can call into S.someCustomizationPoint(), which will
initialize a region of memory in the most efficient way possible,
since it has complete knowledge about the memory layout of S.

Dmitri

···

On Wed, Jul 6, 2016 at 4:21 AM, Karl <razielim@gmail.com> wrote:

I had a PR open for this which added a Collection specialisation, but you
said this could be handled in a more general way which allows for more
optimised mutations. I’m curious how this would work; how does
`RangeReplaceableCollection.replaceSubrange<S:Sequence>(range:, with: S)`
call a more optimised implementation if S also conforms to Collection, if
not by adding a specialisation?

--
main(i,j){for(i=2;;i++){for(j=2;j<i;j++){if(!(i%j)){j=0;break;}}if
(j){printf("%d\n",i);}}} /*Dmitri Gribenko <gribozavr@gmail.com>*/


(Dmitri Gribenko) #12

Hi Harlan,

For this case, Unmanaged is the recommended API.

Dmitri

···

On Wed, Jul 6, 2016 at 11:30 AM, Harlan Haskins <harlan@harlanhaskins.com> wrote:

I’ve also seen unsafeAddressOf(_:slight_smile: used when interfacing with C function
pointers when the lifetime of an object is guaranteed. Many C APIs vend an
API like:

void perform_action(void (*callback)(void *data), void *initial_data);

For which it is expected to use unsafeAddressOf on a class instance, like:

perform_action({ data in
  let _self = unsafeBitCast(data, to: MyClass.self)
  _self.foo()
}, data: unsafeAddressOf(self))

It’s unsafe and error-prone, sure, but that’s why we have `unsafe` in the
name :sweat_smile: — I’ve had to use this to interface with libclang.

--
main(i,j){for(i=2;;i++){for(j=2;j<i;j++){if(!(i%j)){j=0;break;}}if
(j){printf("%d\n",i);}}} /*Dmitri Gribenko <gribozavr@gmail.com>*/


(Karl) #13

Sorry, it’s been a little while since I looked at it (https://github.com/apple/swift/pull/3067). Actually, the issue is with the append() function specifically - it only has a Sequence specialisation. That’s the point; we were losing type information and not calling the more optimised replaceSubrange — which is exactly the specialisation point you are talking about, Dmitry (I think everything funnels down in to replaceSubrange).

I must have misunderstood what you were saying at the time. I’ll have to test, but I think it’s still an issue. I thought there was some more general work on RRC planned, so when I saw the bullet I thought maybe that was it.

@Dave:

I was trying to initialise a String.CharacterView - depending on whether I initialised it with another CharacterView, or created it empty and appended the other CharacterView, I would get huge performance differences, as the other CharacterView would get split up and a new Character created and appended for every character in the string (which had like 10,000 characters). It’s a subtle bug and a bit nasty when it hits you.

Karl

···

On 7 Jul 2016, at 02:06, Dmitri Gribenko <gribozavr@gmail.com> wrote:

On Wed, Jul 6, 2016 at 4:21 AM, Karl <razielim@gmail.com> wrote:

I had a PR open for this which added a Collection specialisation, but you
said this could be handled in a more general way which allows for more
optimised mutations. I’m curious how this would work; how does
`RangeReplaceableCollection.replaceSubrange<S:Sequence>(range:, with: S)`
call a more optimised implementation if S also conforms to Collection, if
not by adding a specialisation?

The RRC can call into S.someCustomizationPoint(), which will
initialize a region of memory in the most efficient way possible,
since it has complete knowledge about the memory layout of S.

Dmitri

--
main(i,j){for(i=2;;i++){for(j=2;j<i;j++){if(!(i%j)){j=0;break;}}if
(j){printf("%d\n",i);}}} /*Dmitri Gribenko <gribozavr@gmail.com>*/


(Dmitri Gribenko) #14

I had a PR open for this which added a Collection specialisation, but you
said this could be handled in a more general way which allows for more
optimised mutations. I’m curious how this would work; how does
`RangeReplaceableCollection.replaceSubrange<S:Sequence>(range:, with: S)`
call a more optimised implementation if S also conforms to Collection, if
not by adding a specialisation?

The RRC can call into S.someCustomizationPoint(), which will
initialize a region of memory in the most efficient way possible,
since it has complete knowledge about the memory layout of S.

Dmitri

--
main(i,j){for(i=2;;i++){for(j=2;j<i;j++){if(!(i%j)){j=0;break;}}if
(j){printf("%d\n",i);}}} /*Dmitri Gribenko <gribozavr@gmail.com>*/

Sorry, it’s been a little while since I looked at it
(https://github.com/apple/swift/pull/3067). Actually, the issue is with the
append() function specifically - it only has a Sequence specialisation.
That’s the point; we were losing type information and not calling the more
optimised replaceSubrange — which is exactly the specialisation point you
are talking about, Dmitry (I think everything funnels down in to
replaceSubrange).

I'm talking about a different customization point. RRC.append() calls
RRC.replaceSubrange(), which, in our thinking, would call
S._copyContents(initializing: RRC.getInnerPointer()). _copyContents()
is the customization point on the argument that would initialize
memory with the contents of the sequence.

I must have misunderstood what you were saying at the time. I’ll have to
test, but I think it’s still an issue. I thought there was some more general
work on RRC planned, so when I saw the bullet I thought maybe that was it.

I will be sending a draft proposal soon that will indirectly fix this
performance issue.

Dmitri

···

On Wed, Jul 6, 2016 at 6:15 PM, Karl <razielim@gmail.com> wrote:

On 7 Jul 2016, at 02:06, Dmitri Gribenko <gribozavr@gmail.com> wrote:
On Wed, Jul 6, 2016 at 4:21 AM, Karl <razielim@gmail.com> wrote:

--
main(i,j){for(i=2;;i++){for(j=2;j<i;j++){if(!(i%j)){j=0;break;}}if
(j){printf("%d\n",i);}}} /*Dmitri Gribenko <gribozavr@gmail.com>*/


(Karl) #15

Ah no, my memory’s coming back - it wasn’t the overhead in the standard library that was the biggest problem, as much as the 10,000 calls to replaceSubrange with a single element being appended. We had to do some additional processing, and it was just killer doing it so many times.

Karl

···

On 7 Jul 2016, at 03:15, Karl <raziel.im+swift-evo@gmail.com> wrote:

On 7 Jul 2016, at 02:06, Dmitri Gribenko <gribozavr@gmail.com <mailto:gribozavr@gmail.com>> wrote:

On Wed, Jul 6, 2016 at 4:21 AM, Karl <razielim@gmail.com <mailto:razielim@gmail.com>> wrote:

I had a PR open for this which added a Collection specialisation, but you
said this could be handled in a more general way which allows for more
optimised mutations. I’m curious how this would work; how does
`RangeReplaceableCollection.replaceSubrange<S:Sequence>(range:, with: S)`
call a more optimised implementation if S also conforms to Collection, if
not by adding a specialisation?

The RRC can call into S.someCustomizationPoint(), which will
initialize a region of memory in the most efficient way possible,
since it has complete knowledge about the memory layout of S.

Dmitri

--
main(i,j){for(i=2;;i++){for(j=2;j<i;j++){if(!(i%j)){j=0;break;}}if
(j){printf("%d\n",i);}}} /*Dmitri Gribenko <gribozavr@gmail.com <mailto:gribozavr@gmail.com>>*/

Sorry, it’s been a little while since I looked at it (https://github.com/apple/swift/pull/3067). Actually, the issue is with the append() function specifically - it only has a Sequence specialisation. That’s the point; we were losing type information and not calling the more optimised replaceSubrange — which is exactly the specialisation point you are talking about, Dmitry (I think everything funnels down in to replaceSubrange).

I must have misunderstood what you were saying at the time. I’ll have to test, but I think it’s still an issue. I thought there was some more general work on RRC planned, so when I saw the bullet I thought maybe that was it.

@Dave:

I was trying to initialise a String.CharacterView - depending on whether I initialised it with another CharacterView, or created it empty and appended the other CharacterView, I would get huge performance differences, as the other CharacterView would get split up and a new Character created and appended for every character in the string (which had like 10,000 characters). It’s a subtle bug and a bit nasty when it hits you.

Karl