From 8ff5da8f76f21c9a6a1a036273ee13f6cd45a531 Mon Sep 17 00:00:00 2001 From: Erik Eckstein Date: Thu, 3 May 2018 14:36:54 -0700 Subject: [PATCH 1/4] stdlib: remove some @inlineables from String API functions. Beside the general goal to remove inlinable functions, this reduces code size and also improves performance for several benchmarks. The performance problem was that by inlining top-level String API functions into client code (like String.count) it ended up calling non-inlinable internal String functions eventually. This is much slower than to make a single call at the top-level API boundary into the library. Inside the library all the internal String functions can be specialized and inlined. rdar://problem/39921548 --- stdlib/public/core/StringGuts.swift | 1 - stdlib/public/core/StringIndexConversions.swift | 1 - stdlib/public/core/StringLegacy.swift | 2 -- .../core/StringRangeReplaceableCollection.swift | 12 +++++------- 4 files changed, 5 insertions(+), 11 deletions(-) diff --git a/stdlib/public/core/StringGuts.swift b/stdlib/public/core/StringGuts.swift index 95eb11b569f29..eec0c8532d9e8 100644 --- a/stdlib/public/core/StringGuts.swift +++ b/stdlib/public/core/StringGuts.swift @@ -1030,7 +1030,6 @@ extension _StringGuts { self.append(other._wholeString._guts, range: other._encodedOffsetRange) } - @inlinable public // TODO(StringGuts): for testing only mutating func append(_ other: _StringGuts) { // FIXME(TODO: JIRA): shouldn't _isEmptySingleton be sufficient? diff --git a/stdlib/public/core/StringIndexConversions.swift b/stdlib/public/core/StringIndexConversions.swift index 15b5a4734d9e7..c97cd873bfda0 100644 --- a/stdlib/public/core/StringIndexConversions.swift +++ b/stdlib/public/core/StringIndexConversions.swift @@ -49,7 +49,6 @@ extension String.Index { /// `sourcePosition` must be a valid index of at least one of the views /// of `target`. /// - target: The string referenced by the resulting index. - @inlinable // FIXME(sil-serialize-all) public init?( _ sourcePosition: String.Index, within target: String diff --git a/stdlib/public/core/StringLegacy.swift b/stdlib/public/core/StringLegacy.swift index 1254207148445..fc365f79bc8b3 100644 --- a/stdlib/public/core/StringLegacy.swift +++ b/stdlib/public/core/StringLegacy.swift @@ -160,7 +160,6 @@ extension StringProtocol { } extension String { - @inlinable // FIXME(sil-serialize-all) public func hasPrefix(_ prefix: String) -> Bool { let prefixCount = prefix._guts.count if prefixCount == 0 { return true } @@ -208,7 +207,6 @@ extension String { return self.starts(with: prefix) } - @inlinable // FIXME(sil-serialize-all) public func hasSuffix(_ suffix: String) -> Bool { let suffixCount = suffix._guts.count if suffixCount == 0 { return true } diff --git a/stdlib/public/core/StringRangeReplaceableCollection.swift b/stdlib/public/core/StringRangeReplaceableCollection.swift index 80e4780444f51..40f5efb481e00 100644 --- a/stdlib/public/core/StringRangeReplaceableCollection.swift +++ b/stdlib/public/core/StringRangeReplaceableCollection.swift @@ -89,6 +89,11 @@ extension String : StringProtocol, RangeReplaceableCollection { @inlinable // FIXME(sil-serialize-all) public var endIndex: Index { return Index(encodedOffset: _guts.count) } + /// The number of characters in a string. + public var count: Int { + return distance(from: startIndex, to: endIndex) + } + @inlinable @inline(__always) internal func _boundsCheck(_ index: Index) { @@ -114,7 +119,6 @@ extension String : StringProtocol, RangeReplaceableCollection { "String index range is out of bounds") } - @inlinable // FIXME(sil-serialize-all) internal func _index(atEncodedOffset offset: Int) -> Index { return _visitGuts(_guts, args: offset, ascii: { ascii, offset in return ascii.characterIndex(atOffset: offset) }, @@ -128,7 +132,6 @@ extension String : StringProtocol, RangeReplaceableCollection { /// - Parameter i: A valid index of the collection. `i` must be less than /// `endIndex`. /// - Returns: The index value immediately after `i`. - @inlinable // FIXME(sil-serialize-all) public func index(after i: Index) -> Index { return _visitGuts(_guts, args: i, ascii: { ascii, i in ascii.characterIndex(after: i) }, @@ -141,7 +144,6 @@ extension String : StringProtocol, RangeReplaceableCollection { /// - Parameter i: A valid index of the collection. `i` must be greater than /// `startIndex`. /// - Returns: The index value immediately before `i`. - @inlinable // FIXME(sil-serialize-all) public func index(before i: Index) -> Index { return _visitGuts(_guts, args: i, ascii: { ascii, i in ascii.characterIndex(before: i) }, @@ -171,7 +173,6 @@ extension String : StringProtocol, RangeReplaceableCollection { /// to `index(before:)`. /// /// - Complexity: O(*n*), where *n* is the absolute value of `n`. - @inlinable // FIXME(sil-serialize-all) public func index(_ i: Index, offsetBy n: IndexDistance) -> Index { return _visitGuts(_guts, args: (i, n), ascii: { ascii, args in let (i, n) = args @@ -219,7 +220,6 @@ extension String : StringProtocol, RangeReplaceableCollection { /// the method returns `nil`. /// /// - Complexity: O(*n*), where *n* is the absolute value of `n`. - @inlinable // FIXME(sil-serialize-all) public func index( _ i: Index, offsetBy n: IndexDistance, limitedBy limit: Index ) -> Index? { @@ -241,7 +241,6 @@ extension String : StringProtocol, RangeReplaceableCollection { /// - Returns: The distance between `start` and `end`. /// /// - Complexity: O(*n*), where *n* is the resulting distance. - @inlinable // FIXME(sil-serialize-all) public func distance(from start: Index, to end: Index) -> IndexDistance { return _visitGuts(_guts, args: (start, end), ascii: { ascii, args in let (start, end) = args @@ -267,7 +266,6 @@ extension String : StringProtocol, RangeReplaceableCollection { /// /// - Parameter i: A valid index of the string. `i` must be less than the /// string's end index. - @inlinable // FIXME(sil-serialize-all) public subscript(i: Index) -> Character { return _visitGuts(_guts, args: i, ascii: { ascii, i in return ascii.character(at: i) }, From d65ac5004cdef7a1aec66539225c12525577adef Mon Sep 17 00:00:00 2001 From: Erik Eckstein Date: Mon, 7 May 2018 14:48:04 -0700 Subject: [PATCH 2/4] stdlib: fix performance regression for long string appends. Re-wrote the inner memcpy loops so that they can be vectorized. Also added a few inline(__always). Since we removed some @inlineable attributes this string-append code is not code generated in the client anymore. The code generation in the stdlib binary is different because all the precondition checks are not folded away. Using explicit loop control statements instead of for-in-range removes the precondition-overhead for those time critical memcpy loops. --- stdlib/public/core/StringStorage.swift | 5 ++++- stdlib/public/core/UnmanagedString.swift | 17 +++++++++++++++-- 2 files changed, 19 insertions(+), 3 deletions(-) diff --git a/stdlib/public/core/StringStorage.swift b/stdlib/public/core/StringStorage.swift index 43fdbe7b7a551..3c19b0c114f0b 100644 --- a/stdlib/public/core/StringStorage.swift +++ b/stdlib/public/core/StringStorage.swift @@ -174,7 +174,10 @@ extension _SwiftStringStorage { @inlinable @nonobjc var unusedBuffer: UnsafeMutableBufferPointer { - return UnsafeMutableBufferPointer(start: end, count: capacity - count) + @inline(__always) + get { + return UnsafeMutableBufferPointer(start: end, count: capacity - count) + } } @inlinable diff --git a/stdlib/public/core/UnmanagedString.swift b/stdlib/public/core/UnmanagedString.swift index 583df043e5b6a..d109c17b301f6 100644 --- a/stdlib/public/core/UnmanagedString.swift +++ b/stdlib/public/core/UnmanagedString.swift @@ -16,6 +16,7 @@ internal typealias _UnmanagedASCIIString = _UnmanagedString internal typealias _UnmanagedUTF16String = _UnmanagedString @inlinable +@inline(__always) internal func memcpy_zext< Target: FixedWidthInteger & UnsignedInteger, @@ -24,12 +25,18 @@ func memcpy_zext< dst: UnsafeMutablePointer, src: UnsafePointer, count: Int ) { _sanityCheck(Source.bitWidth < Target.bitWidth) - for i in 0..= 0) + // Don't use the for-in-range syntax to avoid precondition checking in Range. + // This enables vectorization of the memcpy loop. + var i = 0 + while i < count { dst[i] = Target(src[i]) + i = i &+ 1 } } @inlinable +@inline(__always) internal func memcpy_trunc< Target: FixedWidthInteger & UnsignedInteger, @@ -38,8 +45,13 @@ func memcpy_trunc< dst: UnsafeMutablePointer, src: UnsafePointer, count: Int ) { _sanityCheck(Source.bitWidth > Target.bitWidth) - for i in 0..= 0) + // Don't use the for-in-range syntax to avoid precondition checking in Range. + // This enables vectorization of the memcpy loop. + var i = 0 + while i < count { dst[i] = Target(truncatingIfNeeded: src[i]) + i = i &+ 1 } } @@ -194,6 +206,7 @@ extension _UnmanagedString : _StringVariant { } @inlinable // FIXME(sil-serialize-all) + @inline(__always) internal func _copy( into target: UnsafeMutableBufferPointer ) where TargetCodeUnit : FixedWidthInteger & UnsignedInteger { From 4992ce79078c59bd38efa5500ed0a34235a2dece Mon Sep 17 00:00:00 2001 From: Erik Eckstein Date: Tue, 8 May 2018 10:39:13 -0700 Subject: [PATCH 3/4] stdlib: Speed up UTF8View -> Array conversion by using _copyContents --- .../public/core/ContiguousArrayBuffer.swift | 20 ++++++++++--------- validation-test/stdlib/Arrays.swift.gyb | 15 +++++++------- 2 files changed, 19 insertions(+), 16 deletions(-) diff --git a/stdlib/public/core/ContiguousArrayBuffer.swift b/stdlib/public/core/ContiguousArrayBuffer.swift index 659a54ed2b279..eb9de9f689a88 100644 --- a/stdlib/public/core/ContiguousArrayBuffer.swift +++ b/stdlib/public/core/ContiguousArrayBuffer.swift @@ -642,15 +642,17 @@ internal func _copyCollectionToContiguousArray< _uninitializedCount: count, minimumCapacity: 0) - var p = result.firstElementAddress - var i = source.startIndex - for _ in 0.. Date: Thu, 3 May 2018 17:30:41 -0700 Subject: [PATCH 4/4] [test] Update diagnostic test for SR-7599 --- test/Constraints/diagnostics.swift | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/Constraints/diagnostics.swift b/test/Constraints/diagnostics.swift index 6701990daf576..9f6c0310f51cb 100644 --- a/test/Constraints/diagnostics.swift +++ b/test/Constraints/diagnostics.swift @@ -161,7 +161,8 @@ func rdar20142523() { // Bad diagnostic for invalid method call in boolean expression: (_, ExpressibleByIntegerLiteral)' is not convertible to 'ExpressibleByIntegerLiteral func rdar21080030() { var s = "Hello" - if s.count() == 0 {} // expected-error{{cannot call value of non-function type 'Int'}}{{13-15=}} + // SR-7599: This should be `cannot_call_non_function_value` + if s.count() == 0 {} // expected-error{{cannot invoke 'count' with no arguments}} } // QoI: problem with return type inference mis-diagnosed as invalid arguments