Speedup for NSString -getCharacters:range: #1928
Conversation
|
@swift-ci please test |
|
Do you have a benchmark for this? UBP's subscript is probably too slow, I've found it significantly faster to get the underlying UP and dereference that. |
|
Heh whoops. A little TOO untested there. Will fix it up later. @milseman it shows up on the regex test in the benchmark game |
|
@Catfish-Man Do we have a version of this benchmark? (We should probably add it). |
|
Maybe we even add it before this PR and then we can use the PR to validate the speed up! = ) |
|
@swift-ci please test |
1 similar comment
|
@swift-ci please test |
Catfish-Man
changed the title
|
@swift-ci please test |
1 similar comment
|
@swift-ci please test |
|
I whipped up a "simulated" benchmark for this since I don't have a Linux machine handy, and on my test at least the current patch is >10x faster than the old implementation. Unfortunately we were also seeing crashes in NSKeyedArchiver tests with the previous version of it… we'll see how this one does. |
|
We changed how testing occurs for SCF patches. Restarting test. |
|
@swift-ci please test |
|
Hm. I wonder what "llvm-config: error: component libraries and shared library" means (from the linux build) |
|
@Catfish-Man When running your EXC_BAD_ACCESS in NSString.swift line 349 |
|
Oh excellent, thank you |
|
Ok, finally got myself set up properly to do corelibs debugging rather than stdlib, and I see what's wrong here. Fix shortly, thank you for your patience :) |
|
Should be good to go now. It wasn't handling _NSCFConstantString. |
|
@swift-ci please test |
| @@ -343,8 +343,18 @@ open class NSString : NSObject, NSCopying, NSMutableCopying, NSSecureCoding, NSC | |||
|
|
|||
| extension NSString { | |||
| public func getCharacters(_ buffer: UnsafeMutablePointer<unichar>, range: NSRange) { | |||
| for idx in 0..<range.length { | |||
| buffer[idx] = character(at: idx + range.location) | |||
| if type(of: self) == NSString.self || type(of: self) == NSMutableString.self { | |||
There was a problem hiding this comment.
How much does this type check slow things down? I suppose anything is better than character(at:)...
There was a problem hiding this comment.
There's a lot of things we could do to improve this further. My goal here is just to get it from n^2 to n so that it's not a huge problem. If we find important uses of it we can sink more time into it.
|
@swift-ci please test |
|
You can also get up to 2x improvement on contiguous UTF-8 strings in general and several X when it's ASCII, depending on how many code paths you want to maintain here. Pseudo-code (you'll have to // Fast-path Handle ASCII prefixes
_storage.utf8.withContiguousStorageIfAvailable {
for byte in $0 {
guard byte < 0x80 else { return }
output[i] = UInt16(truncatingIfNeeded: byte)
}
}
// Now we calculate the index corresponding to our (potentially further advanced) offsets... |
No description provided.