Re: [swift-corelibs-dev] IndexPath performance

2016-08-02 Thread Kevin Ballard via swift-corelibs-dev
FWIW, if you agree that this approach is reasonable, I would be willing
to write the patch for it.

-Kevin Ballard

On Tue, Aug 2, 2016, at 05:46 PM, Kevin Ballard wrote:
> I agree with Stephan, NSIndexPath performance is important and we
> should avoid the overhead of allocating/freeing an array for the
> common case. Instead of just always wrapping NSIndexPath, maybe we
> should just switch the internal representation to something like
>
> enum Indices {
> case one(Int)
> case two(Int, Int)
> // case three?
> case many([Int])
> }
>
> Yeah it complicates the methods a bit, and we'd have to have a custom
> index instead of just using Array's index, but it avoids heap
> allocation for the extremely common case of a 2-element index path
> (it's so common, I don't think I've *ever* seen an NSIndexPath that
> didn't contain exactly 2 indices).
>
> -Kevin Ballard
>
> On Tue, Aug 2, 2016, at 04:24 AM, Stephan Tolksdorf via swift-corelibs-
> dev wrote:
>> Tony,
>>
>> I understand why you'd ideally want to have a real-world benchmark to
>> guide performance optimisations, but if you require that for every
>> performance-related change, you set a very high bar, and that bar
>> will probably have the effect of biasing performance downwards, since
>> if there is no existing benchmark, changes that worsen performance
>> might not get flagged.
>>
>> The fact that NSIndexPath got the tagged pointer treatment probably
>> indicates that its implementation has a non-negligible effect on
>> performance (see also
>> https://twitter.com/Catfish_Man/status/393249511075639296).
>>
>> The current IndexPath implementation in terms of an Int array clearly
>> introduces unnecessary overhead in ObjC interop scenarios, so unless
>> this implementation of IndexPath has some benefit I don't understand,
>> I'd argue that it should be replaced with a straightforward wrapper
>> around an NSIndexPath value.
>>
>> - Stephan
>>
>> On 2 August 2016 at 12:12, Tony Parker
>>  wrote:
>>> Hi Stephan,
>>>
>>>
 On Aug 2, 2016, at 6:04 PM, Stephan Tolksdorf 
 wrote:

 Hi Parker,

 I noticed the IndexPath overhead when I investigated why a Swift 3
 implementation of
 UICollectionViewLayout.layoutAttributesForElementsInRect spent more
 time in malloc, free and related methods, but I don't have a
 benchmark.

 Is it important that IndexPath uses native Swift refcounting? It
 seems to me that this type is mainly used in ObjC interop code. In
 native Swift code I would always try to avoid using a dynamically
 sized, heap allocated array as a data structure index. If
 NSIndexPath can't be bridged to a native Swift type without
 introducing additional overhead, then maybe it shouldn't be bridged
 at all?

 - Stephan
>>>
>>>
>>>
>>> I do think it is likely we could figure out some improvements here,
>>> but I’d like to start with a concrete test (and something that is
>>> representative of real world use cases). If it’s possible to extract
>>> something out of what you’ve already done, that would be really
>>> helpful. We can also file a bug on bugs.swift.org as a call for help
>>> designing a better perf test suite (we need this for all of the
>>> types, frankly).
>>>
>>> Once we know we’re measuring the right thing, there are all kinds of
>>> interesting things we can do. If (when?) we have ABI stability in
>>> Swift 4, we may be able to also change the ObjC Foundation.framework
>>> to better cooperate with the Swift side, as we’ll be able to tie the
>>> current overlay code to a specific OS instead of having to run back
>>> several releases.
>>>
>>> Thanks,
>>> - Tony
>>>
>>>

 On 2 August 2016 at 11:09, Tony Parker 
 wrote:
> Hi Stephan,
>
>  Do you have some benchmarks that you could share? That would help
>  us focus performance work in the right area.
>
>  I know that 2-item IndexPaths are super common with UIKit
>  collection view and friends, so we may just want to special case
>  those. Unfortunately, NSIndexPath is not abstract, so subclassing
>  it in the same way that we do for a few of the other bridged
>  types (to use native Swift refcounting) is not easy. On the other
>  hand, the ObjC implementation does use tagged pointers, so some
>  NSIndexPaths are really cheap to create.
>
>  - Tony
>
> > On Aug 1, 2016, at 11:44 PM, Stephan Tolksdorf via swift-corelibs-
> > dev  wrote:
>  >
>  > Hi,
>  >
>  > IndexPath is currently implemented using an [Int] array that is
>  > bridged to an NSIndexPath only on demand. Since IndexPath
>  > values are primarily used together with Objective-C APIs,
>  > wouldn't it be better to implement IndexPath directly as an
>  > NSIndexPath wrapper, in order to avoid the overhead of
>  > temporary array 

Re: [swift-corelibs-dev] IndexPath performance

2016-08-02 Thread Kevin Ballard via swift-corelibs-dev
I agree with Stephan, NSIndexPath performance is important and we should
avoid the overhead of allocating/freeing an array for the common case.
Instead of just always wrapping NSIndexPath, maybe we should just switch
the internal representation to something like

enum Indices {
case one(Int)
case two(Int, Int)
// case three?
case many([Int])
}

Yeah it complicates the methods a bit, and we'd have to have a custom
index instead of just using Array's index, but it avoids heap allocation
for the extremely common case of a 2-element index path (it's so common,
I don't think I've *ever* seen an NSIndexPath that didn't contain
exactly 2 indices).

-Kevin Ballard

On Tue, Aug 2, 2016, at 04:24 AM, Stephan Tolksdorf via swift-corelibs-dev 
wrote:
> Tony,
>
> I understand why you'd ideally want to have a real-world benchmark to
> guide performance optimisations, but if you require that for every 
> performance-
> related change, you set a very high bar, and that bar will probably
> have the effect of biasing performance downwards, since if there is no
> existing benchmark, changes that worsen performance might not get
> flagged.
>
> The fact that NSIndexPath got the tagged pointer treatment probably
> indicates that its implementation has a non-negligible effect on
> performance (see also
> https://twitter.com/Catfish_Man/status/393249511075639296).
>
> The current IndexPath implementation in terms of an Int array clearly
> introduces unnecessary overhead in ObjC interop scenarios, so unless
> this implementation of IndexPath has some benefit I don't understand,
> I'd argue that it should be replaced with a straightforward wrapper
> around an NSIndexPath value.
>
> - Stephan
>
> On 2 August 2016 at 12:12, Tony Parker
>  wrote:
>> Hi Stephan,
>>
>>
>>> On Aug 2, 2016, at 6:04 PM, Stephan Tolksdorf 
>>> wrote:
>>>
>>> Hi Parker,
>>>
>>> I noticed the IndexPath overhead when I investigated why a Swift 3
>>> implementation of
>>> UICollectionViewLayout.layoutAttributesForElementsInRect spent more
>>> time in malloc, free and related methods, but I don't have a
>>> benchmark.
>>>
>>> Is it important that IndexPath uses native Swift refcounting? It
>>> seems to me that this type is mainly used in ObjC interop code. In
>>> native Swift code I would always try to avoid using a dynamically
>>> sized, heap allocated array as a data structure index. If
>>> NSIndexPath can't be bridged to a native Swift type without
>>> introducing additional overhead, then maybe it shouldn't be bridged
>>> at all?
>>>
>>> - Stephan
>>
>>
>>
>> I do think it is likely we could figure out some improvements here,
>> but I’d like to start with a concrete test (and something that is
>> representative of real world use cases). If it’s possible to extract
>> something out of what you’ve already done, that would be really
>> helpful. We can also file a bug on bugs.swift.org as a call for help
>> designing a better perf test suite (we need this for all of the
>> types, frankly).
>>
>> Once we know we’re measuring the right thing, there are all kinds of
>> interesting things we can do. If (when?) we have ABI stability in
>> Swift 4, we may be able to also change the ObjC Foundation.framework
>> to better cooperate with the Swift side, as we’ll be able to tie the
>> current overlay code to a specific OS instead of having to run back
>> several releases.
>>
>> Thanks,
>> - Tony
>>
>>
>>>
>>> On 2 August 2016 at 11:09, Tony Parker 
>>> wrote:
 Hi Stephan,

  Do you have some benchmarks that you could share? That would help
  us focus performance work in the right area.

  I know that 2-item IndexPaths are super common with UIKit
  collection view and friends, so we may just want to special case
  those. Unfortunately, NSIndexPath is not abstract, so subclassing
  it in the same way that we do for a few of the other bridged types
  (to use native Swift refcounting) is not easy. On the other hand,
  the ObjC implementation does use tagged pointers, so some
  NSIndexPaths are really cheap to create.

  - Tony

 > On Aug 1, 2016, at 11:44 PM, Stephan Tolksdorf via swift-corelibs-
 > dev  wrote:
  >
  > Hi,
  >
  > IndexPath is currently implemented using an [Int] array that is
  > bridged to an NSIndexPath only on demand. Since IndexPath values
  > are primarily used together with Objective-C APIs, wouldn't it
  > be better to implement IndexPath directly as an NSIndexPath
  > wrapper, in order to avoid the overhead of temporary array
  > instances?
  >
  > - Stephan
 > ___
  > swift-corelibs-dev mailing list swift-corelibs-dev@swift.org
  > https://lists.swift.org/mailman/listinfo/swift-corelibs-dev
>>>
>>
> _
> swift-corelibs-dev 

[swift-corelibs-dev] Query on init methods in Data.swift

2016-08-02 Thread Simon Evans via swift-corelibs-dev
Hi

I was looking at Data.swift and noticed that 2 of the init methods
were different in corelibs-foundation v swift stdlib

https://github.com/apple/swift-corelibs-foundation/blob/master/Foundation/Data.swift
 has

public init?(capacity: Int)
public init?(count: Int)

https://github.com/apple/swift/blob/master/stdlib/public/SDK/Foundation/Data.swift
 has

public init(capacity: Int)
public init(count: Int)


Which are the correct method signatures? I was just doing a PR to fix
init?(count:) not zeroing the data and I wanted to make sure I had this
correct as well

Thanks
Simon
___
swift-corelibs-dev mailing list
swift-corelibs-dev@swift.org
https://lists.swift.org/mailman/listinfo/swift-corelibs-dev


Re: [swift-corelibs-dev] IndexPath performance

2016-08-02 Thread Stephan Tolksdorf via swift-corelibs-dev
Oh, I'm sorry, Tony, I was too hasty and mistook your last name for your
first name :-(

- Stephan

On 2 August 2016 at 12:04, Stephan Tolksdorf  wrote:

> Hi Parker,
>
> I noticed the IndexPath overhead when I investigated why a Swift 3
> implementation of UICollectionViewLayout.layoutAttributesForElementsInRect
> spent more time in malloc, free and related methods, but I don't have a
> benchmark.
>
> Is it important that IndexPath uses native Swift refcounting? It seems to
> me that this type is mainly used in ObjC interop code. In native Swift code
> I would always try to avoid using a dynamically sized, heap allocated array
> as a data structure index. If NSIndexPath can't be bridged to a native
> Swift type without introducing additional overhead, then maybe it shouldn't
> be bridged at all?
>
> - Stephan
>
> On 2 August 2016 at 11:09, Tony Parker  wrote:
>
>> Hi Stephan,
>>
>> Do you have some benchmarks that you could share? That would help us
>> focus performance work in the right area.
>>
>> I know that 2-item IndexPaths are super common with UIKit collection view
>> and friends, so we may just want to special case those. Unfortunately,
>> NSIndexPath is not abstract, so subclassing it in the same way that we do
>> for a few of the other bridged types (to use native Swift refcounting) is
>> not easy. On the other hand, the ObjC implementation does use tagged
>> pointers, so some NSIndexPaths are really cheap to create.
>>
>> - Tony
>>
>> > On Aug 1, 2016, at 11:44 PM, Stephan Tolksdorf via swift-corelibs-dev <
>> swift-corelibs-dev@swift.org> wrote:
>> >
>> > Hi,
>> >
>> > IndexPath is currently implemented using an [Int] array that is bridged
>> to an NSIndexPath only on demand. Since IndexPath values are primarily used
>> together with Objective-C APIs, wouldn't it be better to implement
>> IndexPath directly as an NSIndexPath wrapper, in order to avoid the
>> overhead of temporary array instances?
>> >
>> > - Stephan
>> > ___
>> > swift-corelibs-dev mailing list
>> > swift-corelibs-dev@swift.org
>> > https://lists.swift.org/mailman/listinfo/swift-corelibs-dev
>>
>>
>
___
swift-corelibs-dev mailing list
swift-corelibs-dev@swift.org
https://lists.swift.org/mailman/listinfo/swift-corelibs-dev


Re: [swift-corelibs-dev] IndexPath performance

2016-08-02 Thread Stephan Tolksdorf via swift-corelibs-dev
Hi Parker,

I noticed the IndexPath overhead when I investigated why a Swift 3
implementation of UICollectionViewLayout.layoutAttributesForElementsInRect
spent more time in malloc, free and related methods, but I don't have a
benchmark.

Is it important that IndexPath uses native Swift refcounting? It seems to
me that this type is mainly used in ObjC interop code. In native Swift code
I would always try to avoid using a dynamically sized, heap allocated array
as a data structure index. If NSIndexPath can't be bridged to a native
Swift type without introducing additional overhead, then maybe it shouldn't
be bridged at all?

- Stephan

On 2 August 2016 at 11:09, Tony Parker  wrote:

> Hi Stephan,
>
> Do you have some benchmarks that you could share? That would help us focus
> performance work in the right area.
>
> I know that 2-item IndexPaths are super common with UIKit collection view
> and friends, so we may just want to special case those. Unfortunately,
> NSIndexPath is not abstract, so subclassing it in the same way that we do
> for a few of the other bridged types (to use native Swift refcounting) is
> not easy. On the other hand, the ObjC implementation does use tagged
> pointers, so some NSIndexPaths are really cheap to create.
>
> - Tony
>
> > On Aug 1, 2016, at 11:44 PM, Stephan Tolksdorf via swift-corelibs-dev <
> swift-corelibs-dev@swift.org> wrote:
> >
> > Hi,
> >
> > IndexPath is currently implemented using an [Int] array that is bridged
> to an NSIndexPath only on demand. Since IndexPath values are primarily used
> together with Objective-C APIs, wouldn't it be better to implement
> IndexPath directly as an NSIndexPath wrapper, in order to avoid the
> overhead of temporary array instances?
> >
> > - Stephan
> > ___
> > swift-corelibs-dev mailing list
> > swift-corelibs-dev@swift.org
> > https://lists.swift.org/mailman/listinfo/swift-corelibs-dev
>
>
___
swift-corelibs-dev mailing list
swift-corelibs-dev@swift.org
https://lists.swift.org/mailman/listinfo/swift-corelibs-dev


Re: [swift-corelibs-dev] [SR-1608]

2016-08-02 Thread Tony Parker via swift-corelibs-dev
Hi David,

Thanks for helping!

Can you check out some of our current open PRs to make sure your work isn’t 
overlapping? We’ve had some contributions in this area recently that we still 
need to finish merging.

- Tony

> On Jul 29, 2016, at 11:28 PM, David Liu via swift-corelibs-dev 
>  wrote:
> 
> Hi all,
> 
> I am new to the mailing list, I am  interested in contributing to Swift Open 
> source.
> Recently picked up SR-1608 and have a PR out for it (i split it in 2 to keep 
> the pulls small).
> The code contribution guide lines suggested to contact the mailing list to 
> make sure work is not duplicated.
> So i want to just check with in you guys about it and any guidance is much 
> appreciated.
> 
> Cheers
> 
> Dave
> 
> ___
> swift-corelibs-dev mailing list
> swift-corelibs-dev@swift.org
> https://lists.swift.org/mailman/listinfo/swift-corelibs-dev

___
swift-corelibs-dev mailing list
swift-corelibs-dev@swift.org
https://lists.swift.org/mailman/listinfo/swift-corelibs-dev