Performance improvements #230
Conversation
|
@yonaskolb if you have a chance could you please try this branch with your benchmark or add measurement tests if you have written any already? |
|
@ilyapuchka I ran the same tests again and got great results! Nice work! 🎉 |
|
@yonaskolb thanks! seems acceptable so we can merge it then and someone makes a patch release? |
|
it would be better to include some measurement tests though while we are on that, to avoid such things in future |
|
|
||
| self.lines = templateString.components(separatedBy: .newlines).enumerated().flatMap { | ||
| guard !$0.element.isEmpty else { return nil } | ||
| return (content: $0.element, number: UInt($0.offset + 1), templateString.range(of: $0.element)!) |
There was a problem hiding this comment.
Catching up and reading that PR content a little late…
@ilyapuchka @djbe Doesn't that templateString.range(of: $0.element) introduce the risk that you get the wrong range if the template have multiple similar lines? And also, searching the whole templateString for that $0.element might take some time especially for long templateString strings?
Shouldn't we instead keep a running total of characters up to each line, and construct the range from range = (currentCharIndex..<currentCharIndex + $0.element.length) then curentCharIndex = thatRange.endIndex+1 or something similar to avoid such risk?
Trying to address performance issues introduced by #167