Benchmarks for dropLast and suffix

[therealbnut]

Adds benchmarks testing dropLast and suffix, so we have a baseline for optimisations and improvements.

Note: the various arrays seemed to be in alphabetical order, with a few exceptions. I sorted those where it seemed like it was meant to be. I'm happy to revert that.

This is in order to facilitate #7250

It also will allow us to determine the efficiency of the non-specialised AnySequence implementations. For example:

 54 DropLastAnySequence             3   31460   31927    31695      0      31695    4603904
 55 DropLastCountableRange          3     800     809      804      0        804    4575232
 56 DropLastSequence                3   67560   68419    68100      0      68100    4934315

Ideally DropLastAnySequence would perform similarly to DropLastCountableRange (as, in this case, it wraps a RandomAccessCollection and could find the dropped range in constant time). Likewise for SuffixCountableRange.

Helps benchmark SR-413.

[therealbnut]

CC @dabrahams - as discussed in #7250

[airspeedswift]

The AnySequence perf's pretty disappointing, it's definitely dispatching to the underlying type's implementation so maybe it's specialization that's failing.

Other than that, these look good. Would you mind adding one for Array too?

[therealbnut]

@airspeedswift added array benchmarks in c6dc0b0

 DropLastAnySequence             3   33643   37585    35984      0      35984    4606635
+DropLastArray                   3    1114    1215     1148      0       1148    4609365
 DropLastCountableRange          3     774     783      779      0        779    4517888
 DropLastSequence                3   65900   66555    66242      0      66242    4722688

and

 SuffixAnySequence               3   36442   38835    37366      0      37366    4651691
+SuffixArray                     3    1196    1214     1204      0       1204    4614827
 SuffixCountableRange            3     832     864      851      0        851    4573867
 SuffixSequence                  3   61049   61647    61296      0      61296    4728149

Sorry for the delay!

[airspeedswift]

@swift-ci Please smoke test

[airspeedswift]

@swift-ci Please benchmark

[therealbnut]

thanks :)

[swift-ci]

Build comment file:

Optimized (O)

---

[therealbnut]

@airspeedswift just merged master and fixed conflicts, I think I've addressed all your feedback, was this everything? :)

[lplarson]

@swift-ci benchmark

[lplarson]

@swift-ci benchmark

[lplarson]

@swift-ci smoke test

[swift-ci]

Build comment file:

Build failed before running benchmark.

---

[lplarson]

I'll merge this once we get a clean benchmark run. Blocked on #7979.

[lplarson]

@swift-ci benchmark

[swift-ci]

Build comment file:

Optimized (O)

---

[therealbnut]

Merged master, this should include the #7979 fix now, thanks @lplarson

[CodaFi]

(Drive-by benchmarking; don't mind me)

@swift-ci benchmark

(Benchmark happening here) // @shahmishal

[swift-ci]

!!! Couldn't read commit file !!!

[therealbnut]

@CodaFi I think there was a problem with the CI system on your drive-by benchmark, you may need to re-run it

Slave went offline during the build

[shahmishal]

@swift-ci benchmark

[swift-ci]

Build comment file:

Optimized (O)

---

[lplarson]

@swift-ci smoke test and merge

[lplarson]

@swift-ci smoke test and merge

[gottesmm]

@lplarson Next time can we do a rebase so we don't have merges in the commit stream?

[therealbnut]

@gottesmm You can change GitHub settings to prevent merge commits, squash PRs by default, make master ff-only, and require CI. The squash removes any merge commits.

[therealbnut]

I think these settings would work well given the current workflow:

https://github.com/apple/swift/settings

https://github.com/apple/swift/settings/branches/master

[therealbnut]

It makes many of the same restrictions if you're pushing using git from the terminal

[therealbnut]

Thank you @airspeedswift for your review and @lplarson for your persistence in getting it through CI and merged 💖

[gottesmm]

I am fine with merge commits. Just not on the topic branch itself.

…

Sent from my iPhone

On Mar 14, 2017, at 2:51 PM, Andrew Bennett ***@***.***> wrote: @gottesmm You can change GitHub settings to prevent merge commits, squash PRs by default, make master ff-only, and require CI — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

[palimondo]

@therealbnut said in initial comment:

Ideally DropLastAnySequence would perform similarly to DropLastCountableRange

That is not possible. The way the test is written, MySequence wraps the CountableRange without forwarding to its suffix implementation - thereby explicitly defaulting to the implementation on Sequence. The issue reported in SR-413 was fixed a year ago (see the issue for details). cc @airspeedswift

[therealbnut]

@palimondo: Note, my comment was for DropLastAnySequence (which uses AnySequence) not DropLastSequence (which uses MySequence).

As you can see in the benchmarks run here, it is not fixed in February this year:

SuffixAnySequence               3   36442   38835    37366      0      37366    4651691
SuffixArray                     3    1196    1214     1204      0       1204    4614827

[therealbnut]

The test intentionally does not forward to Array's suffix implementation in order to call the unspecialised defaulted suffix.

• DropLastSequence benchmark tests the performance of the default implementation on Sequence
• DropLastCountableRange benchmark tests the performance of the specialised implementation on CountableRange
• DropLastAnySequence benchmark tests the performance of AnySequence which will fall somewhere around DropLastSequence at the moment, but ideally it would use CountableRange's specialisation and perform similarly. To do this it needs generic magic that Swift doesn't have yet afaik.

Until AnySequence calls through to the specialised implementation SR-413 is not considered to be fixed.

[dabrahams]

I've done some work in the unicode-rethink branch to simplify forwarding collection APIs and to ensure that all customization points actually get forwarded: https://github.com/apple/swift/blob/unicode-rethink/stdlib/public/core/ForwardingWrapper.swift.gyb

Also, I've been building a new suite of collection wrappers for unicode support using techniques that could greatly benefit AnyCollection et al. by avoiding the need for a dynamically-allocated box in many cases: https://github.com/apple/swift/blob/unicode-rethink/stdlib/public/core/AnyUnicode.swift.gyb#L418

On that basis, I would not invest too much energy in tuning the performance of the existing implementations.

[therealbnut]

Sounds great!

[palimondo]

@therealbnut AnySequence does call through to the specialized implementation on the underlying collection. The performance issue you are seeing is due to iterating over elements through the _IteratorBox using witness table of the generic protocol implementation.

Coincidentally the remaining number of elements in the suffix were just so that it gave similar timing as default implementation on the Sequence. I’ve marked SR-413 as Resolved:Done and opened SR-4499 to track the underlying performance issue in existential collections.