[SE-0225] Implementation of isMultiple for BinaryInteger. #18689

stephentyrone · 2018-08-14T00:02:33Z

Implementation tracking proposal for SE-0225.

Preliminary discussion here: https://forums.swift.org/t/even-and-odd-integers/11774/129
Proposal here: swiftlang/swift-evolution#891

Final decision: https://forums.swift.org/t/accepted-with-modifications-se-0225-adding-iseven-isodd-ismultiple-to-binaryinteger/15689

stephentyrone · 2018-08-14T00:02:44Z

@swift-ci please test.

stephentyrone · 2018-08-14T01:20:49Z

@swift-ci please smoke test

stephentyrone · 2018-08-14T01:21:32Z

test/stdlib/Integers.swift.gyb

@@ -560,7 +564,7 @@ tests.test("truncatingIfNeeded") {

 tests.test("Parsing/LosslessStringConvertible") {
  func _toArray<T: LosslessStringConvertible>(_ text: String) -> [T] {
-    return text.split(separator: " ").map { T(String($0)) }.flatMap { $0 }
+    return text.split(separator: " ").map { T(String($0)) }.compactMap { $0 }


Minor update to silence warning while we're here.

unnamedd · 2018-08-14T07:57:35Z

stdlib/public/core/Integers.swift.gyb

+  @inlinable
+  public func isMultiple(of other: Self) -> Bool {
+    // Nothing but zero is a multiple of zero.
+    if other == 0 { return self == 0 }


Why not use of guard for an early return?

In this case I think it reads a lot more clearly as an if, because it avoids the double-negative.

Make sense! @stephentyrone thanks for your response 😉

stephentyrone · 2018-08-14T14:11:33Z

@swift-ci Please smoke test

stephentyrone · 2018-08-14T15:18:44Z

@swift-ci Please smoke test.

stephentyrone · 2018-08-14T15:20:34Z

test/Prototypes/DoubleWidth.swift.gyb

@@ -39,8 +39,7 @@ import StdlibUnittest
 /// integer type. Nesting `DoubleWidth` instances, in particular, may result in
 /// undesirable performance.
 public struct DoubleWidth<Base : FixedWidthInteger>
-  where Base.Magnitude : UnsignedInteger,
-  Base.Words : Collection, Base.Magnitude.Words : Collection {
+  where Base.Words : Collection, Base.Magnitude.Words : Collection {


An unrelated PR a few weeks ago removed the need for the .Magnitude : UnsignedInteger conformance. Removing it to get rid of warning in tests.

stephentyrone · 2018-08-14T15:20:59Z

test/Prototypes/DoubleWidth.swift.gyb

@@ -282,7 +281,7 @@ extension DoubleWidth.Words: Collection {

  public func index(after i: Index) -> Index {
    switch i._value {
-    case let .low(li) where Base.bitWidth < UInt.bitWidth:
+    case .low where Base.bitWidth < UInt.bitWidth:


This change is also just cleaning up some noise.

stephentyrone · 2018-08-31T15:25:16Z

SE-0225 has been approved with revisions. I will update this PR to match what was approved, but I'm not going to pursue merging it until two much more involved changes to integer protocols land, to avoid causing any merge issues for them:

A default implementation is provided for FixedWidthInteger. Very basic test coverage included as well.

stephentyrone · 2018-09-03T19:20:39Z

@swift-ci please test

stephentyrone · 2018-09-03T19:21:26Z

@natecook1000 Either notes on documentation or a follow-on commit appreciated.

swift-ci · 2018-09-03T19:22:29Z

Build failed
Swift Test Linux Platform
Git Sha - eaa9f82c5036aa3839eae6ecae0ab47ed3b5c63c

swift-ci · 2018-09-03T19:22:32Z

Build failed
Swift Test OS X Platform
Git Sha - eaa9f82c5036aa3839eae6ecae0ab47ed3b5c63c

stephentyrone · 2018-09-03T19:27:06Z

@swift-ci please benchmark

swift-ci · 2018-09-03T20:11:09Z

Build comment file:

Performance: -O

TEST	OLD	NEW	DELTA	SPEEDUP
Regression
EqualStringSubstring	45	49	+8.9%	0.92x
CStringLongAscii	3280	3551	+8.3%	0.92x
Improvement
StringHashing_abnormal	1563	1358	-13.1%	1.15x
StringHashing_longSharedPrefix	8106	7436	-8.3%	1.09x
StringHashing_fastPrenormal	8812	8131	-7.7%	1.08x

Code size: -O

TEST	OLD	NEW	DELTA	SPEEDUP
Regression
BinaryFloatingPointConversionFromBinaryInteger.o	12066	12306	+2.0%	0.98x

Performance: -Osize

TEST	OLD	NEW	DELTA	SPEEDUP
Improvement
StringHashing_abnormal	1540	1337	-13.2%	1.15x
StringHashing_longSharedPrefix	8053	7439	-7.6%	1.08x

Code size: -Osize

TEST	OLD	NEW	DELTA	SPEEDUP
Regression
BinaryFloatingPointConversionFromBinaryInteger.o	11778	12018	+2.0%	0.98x

Performance: -Onone

TEST	OLD	NEW	DELTA	SPEEDUP
Regression
PrefixArrayLazy	31392	34843	+11.0%	0.90x
DropFirstArrayLazy	31766	34857	+9.7%	0.91x
SortStringsUnicode	2713	2924	+7.8%	0.93x (?)
Improvement
StringHashing_abnormal	1610	1456	-9.6%	1.11x
ArrayOfPOD	859	783	-8.8%	1.10x (?)

How to read the data

The tables contain differences in performance which are larger than 8% and differences in code size which are larger than 1%.

If you see any unexpected regressions, you should consider fixing the regressions before you merge the PR.

Noise: Sometimes the performance results (not code size!) contain false alarms. Unexpected regressions which are marked with '(?)' are probably noise. If you see regressions which you cannot explain you can try to run the benchmarks again. If regressions still show up, please consult with the performance team (@eeckstein).

Hardware Overview

  Model Name: Mac Pro
  Model Identifier: MacPro6,1
  Processor Name: 12-Core Intel Xeon E5
  Processor Speed: 2.7 GHz
  Number of Processors: 1
  Total Number of Cores: 12
  L2 Cache (per Core): 256 KB
  L3 Cache: 30 MB
  Memory: 64 GB

xwu · 2018-09-03T22:44:55Z

stdlib/public/core/Integers.swift

+    // Nothing but zero is a multiple of zero.
+    if other == 0 { return self == 0 }
+    // Special case to avoid overflow on .min / -1 for signed types.
+    if Self.isSigned && self == .min && other == -1 { return true }


I'd be curious if performance changes if the two conditionals were replaced with a check of the magnitude, i.e. return other.magnitude <= 1 || self % other == 0...

For concrete types, there should basically be no difference in the face of optimization. We should be able to teach the optimizer to have only a single test on the fast-path, but that's follow-on work after the API is in.

stephentyrone · 2018-09-04T13:03:59Z

@swift-ci please smoke test

stephentyrone commented Aug 14, 2018

View reviewed changes

unnamedd reviewed Aug 14, 2018

View reviewed changes

stephentyrone commented Aug 14, 2018

View reviewed changes

Implement SE-0225 (BinaryInteger.isMultiple(of:))

03d8ce5

A default implementation is provided for FixedWidthInteger. Very basic test coverage included as well.

stephentyrone force-pushed the even-odd-integers branch from eaa9f82 to 03d8ce5 Compare September 3, 2018 19:18

stephentyrone changed the title ~~[DNM] Implementation of isEven/isOdd/isMultiple for BinaryInteger.~~ [SE-0225] Implementation of isMultiple for BinaryInteger. Sep 3, 2018

xwu reviewed Sep 3, 2018

View reviewed changes

Minor simplificiation.

ea90bcb

stephentyrone merged commit c1f25e0 into swiftlang:master Sep 4, 2018

stephentyrone deleted the even-odd-integers branch September 4, 2018 14:31

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[SE-0225] Implementation of isMultiple for BinaryInteger. #18689

[SE-0225] Implementation of isMultiple for BinaryInteger. #18689

stephentyrone commented Aug 14, 2018 •

edited

Loading

stephentyrone commented Aug 14, 2018

stephentyrone commented Aug 14, 2018

stephentyrone Aug 14, 2018

unnamedd Aug 14, 2018

stephentyrone Aug 14, 2018 •

edited

Loading

unnamedd Aug 14, 2018

stephentyrone commented Aug 14, 2018

stephentyrone commented Aug 14, 2018

stephentyrone Aug 14, 2018

stephentyrone Aug 14, 2018

stephentyrone commented Aug 31, 2018

stephentyrone commented Sep 3, 2018

stephentyrone commented Sep 3, 2018

swift-ci commented Sep 3, 2018

swift-ci commented Sep 3, 2018

stephentyrone commented Sep 3, 2018

swift-ci commented Sep 3, 2018

xwu Sep 3, 2018

stephentyrone Sep 4, 2018 •

edited

Loading

stephentyrone commented Sep 4, 2018

[SE-0225] Implementation of isMultiple for BinaryInteger. #18689

[SE-0225] Implementation of isMultiple for BinaryInteger. #18689

Conversation

stephentyrone commented Aug 14, 2018 • edited Loading

stephentyrone commented Aug 14, 2018

stephentyrone commented Aug 14, 2018

stephentyrone Aug 14, 2018

Choose a reason for hiding this comment

unnamedd Aug 14, 2018

Choose a reason for hiding this comment

stephentyrone Aug 14, 2018 • edited Loading

Choose a reason for hiding this comment

unnamedd Aug 14, 2018

Choose a reason for hiding this comment

stephentyrone commented Aug 14, 2018

stephentyrone commented Aug 14, 2018

stephentyrone Aug 14, 2018

Choose a reason for hiding this comment

stephentyrone Aug 14, 2018

Choose a reason for hiding this comment

stephentyrone commented Aug 31, 2018

stephentyrone commented Sep 3, 2018

stephentyrone commented Sep 3, 2018

swift-ci commented Sep 3, 2018

swift-ci commented Sep 3, 2018

stephentyrone commented Sep 3, 2018

swift-ci commented Sep 3, 2018

Build comment file:

Performance: -O

Code size: -O

Performance: -Osize

Code size: -Osize

Performance: -Onone

xwu Sep 3, 2018

Choose a reason for hiding this comment

stephentyrone Sep 4, 2018 • edited Loading

Choose a reason for hiding this comment

stephentyrone commented Sep 4, 2018

stephentyrone commented Aug 14, 2018 •

edited

Loading

stephentyrone Aug 14, 2018 •

edited

Loading

stephentyrone Sep 4, 2018 •

edited

Loading