Skip to content

[5.1] [SR-7749] Poor performance of String.replacingOccurrences(of:wi… #2536

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Oct 15, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 36 additions & 8 deletions Foundation/NSString.swift
Original file line number Diff line number Diff line change
Expand Up @@ -1452,6 +1452,18 @@ extension NSMutableString {
}
return 0
}

private static func makeFindResultsRangeIterator(findResults: CFArray, count: Int, backwards: Bool) -> AnyIterator<NSRange> {
var index = 0
return AnyIterator<NSRange>() { () -> NSRange? in
defer { index += 1 }
if index < count {
let rangePtr = CFArrayGetValueAtIndex(findResults, backwards ? count - index - 1 : index)
return NSRange(rangePtr!.load(as: CFRange.self))
}
return nil
}
}

public func replaceOccurrences(of target: String, with replacement: String, options: CompareOptions = [], range searchRange: NSRange) -> Int {
let backwards = options.contains(.backwards)
Expand All @@ -1462,19 +1474,35 @@ extension NSMutableString {
if options.contains(.regularExpression) {
return _replaceOccurrencesOfRegularExpressionPattern(target, withTemplate:replacement, options:options, range: searchRange)
}


if let findResults = CFStringCreateArrayWithFindResults(kCFAllocatorSystemDefault, _cfObject, target._cfObject, CFRange(searchRange), options._cfValue(true)) {
let numOccurrences = CFArrayGetCount(findResults)
for cnt in 0..<numOccurrences {
let rangePtr = CFArrayGetValueAtIndex(findResults, backwards ? cnt : numOccurrences - cnt - 1)
replaceCharacters(in: NSRange(rangePtr!.load(as: CFRange.self)), with: replacement)
guard let findResults = CFStringCreateArrayWithFindResults(kCFAllocatorSystemDefault, _cfObject, target._cfObject, CFRange(searchRange), options._cfValue(true)) else {
return 0
}
let numOccurrences = CFArrayGetCount(findResults)

let rangeIterator = NSMutableString.makeFindResultsRangeIterator(findResults: findResults, count: numOccurrences, backwards: backwards)

guard type(of: self) == NSMutableString.self else {
// If we're dealing with non NSMutableString, mutations must go through `replaceCharacters` (documented behavior)
for range in rangeIterator {
replaceCharacters(in: range, with: replacement)
}

return numOccurrences
} else {
return 0
}

var newStorage = Substring()
var sourceStringCurrentIndex = _storage.startIndex
for range in rangeIterator {
let matchStartIndex = String.Index(utf16Offset: range.location, in: _storage)
let matchEndIndex = String.Index(utf16Offset: range.location + range.length, in: _storage)
newStorage += _storage[sourceStringCurrentIndex ..< matchStartIndex]
newStorage += replacement
sourceStringCurrentIndex = matchEndIndex
}
newStorage += _storage[sourceStringCurrentIndex ..< _storage.endIndex]
_storage = String(newStorage)
return numOccurrences
}

public func applyTransform(_ transform: String, reverse: Bool, range: NSRange, updatedRange resultingRange: NSRangePointer?) -> Bool {
Expand Down
58 changes: 58 additions & 0 deletions TestFoundation/TestNSString.swift
Original file line number Diff line number Diff line change
Expand Up @@ -1413,6 +1413,64 @@ extension TestNSString {
XCTAssertEqual(str4.replacingOccurrences(of: "\n\r", with: " "), "Hello\r\rworld.")
}

func test_replacingOccurrencesInSubclass() {
class TestMutableString: NSMutableString {
private var wrapped: NSMutableString
var replaceCharactersCount: Int = 0

override var length: Int {
return wrapped.length
}

override func character(at index: Int) -> unichar {
return wrapped.character(at: index)
}

override func replaceCharacters(in range: NSRange, with aString: String) {
defer { replaceCharactersCount += 1 }
wrapped.replaceCharacters(in: range, with: aString)
}

override func mutableCopy(with zone: NSZone? = nil) -> Any {
return wrapped.mutableCopy()
}

required init(stringLiteral value: StaticString) {
wrapped = .init(stringLiteral: value)
super.init(stringLiteral: value)
}

required init(capacity: Int) {
fatalError("init(capacity:) has not been implemented")
}

required init(string aString: String) {
fatalError("init(string:) has not been implemented")
}

required convenience init?(coder aDecoder: NSCoder) {
fatalError("init(coder:) has not been implemented")
}

required init(characters: UnsafePointer<unichar>, length: Int) {
fatalError("init(characters:length:) has not been implemented")
}

required convenience init(extendedGraphemeClusterLiteral value: StaticString) {
fatalError("init(extendedGraphemeClusterLiteral:) has not been implemented")
}

required convenience init(unicodeScalarLiteral value: StaticString) {
fatalError("init(unicodeScalarLiteral:) has not been implemented")
}
}

let testString = TestMutableString(stringLiteral: "ababab")
XCTAssertEqual(testString.replacingOccurrences(of: "ab", with: "xx"), "xxxxxx")
XCTAssertEqual(testString.replaceCharactersCount, 3)
}


func test_fileSystemRepresentation() {
let name = "☃" as NSString
let result = name.fileSystemRepresentation
Expand Down