Why string addition takes so long to build?
Asked Answered
P

5

3

I am adding text in UIlabel, & its cost to performance(i have used build time analyser using this link). how can i optimise this code ?

for value in model?.offerings ?? [] {
    offeringsLabel.text = offeringsLabel.text! + " | " +  (value.name ?? "") + "," +  (value.type ?? "")//this addition cost to performance
}

I also tried [ array].joined but that doesn't make any diffrence

Platinize answered 30/11, 2018 at 11:38 Comment(4)
Mostly it's ?? — it has some problems with type inference checks during build. For reference, check here.Larcher
@user28434 you are correct. but model?.offerings is coming from webservice. & its optional. so Any workaround in defining in codable ? i am using - struct Response:Codable { var offerings:[offerings]?Platinize
You have 2 more ?? in string concatenation line too. And you have just to help type inference, by extracting snippets with ?? to variables and explicitly provide the type you're expecting.Larcher
Use guards so you don't have all those optionals. Then it will perform a lot better.Incurvate
S
7

First, to the underlying question. Why is it slow? Chained + is the single most common cause of massive compile-time performance issues in my experience. It's because + has a lot of overloads (I count 103 in 4.2's stdlib). Swift doesn't just have to prove that + can be used on (String, String) as you want it to be. It has to prove there is no other possible overload combination that is equally valid (if there were, it'd be ambiguous and an error). That doesn't just include simple cases like (Int, Int). It also includes complicated protocol-based overloads that apply to (String, String), but aren't as precise as (String, String) like:

static func + <Other>(lhs: String, rhs: Other) -> String 
    where Other : RangeReplaceableCollection, Character == Other.Element

This takes a long time because it's combinatorially explosive when you have a bunch of + chained together. Each sub-expression has to be reconsidered in light of everything it might return.

How to fix it? First, I wouldn't use a for loop in this case to build up a string piece by piece. Just map each Offering to the string you want, and then join them together:

offeringsLabel.text = model?.offerings
    .map { "\($0.name ?? ""),\($0.type ?? "")" }
    .joined(separator: " | ")

Not only will this compile much more quickly, but it's IMO much clearer what you're doing. It also doesn't require a !, which is nice. (There are other ways to fix that, but it's a nice side-effect).

That said, this is also pointing to a probable issue in your model. It's a separate issue, but I would still take it seriously. Anytime you have code that looks like:

optionalString ?? ""

you need to ask, should this really be Optional? Do you really treat a nil name differently than an empty name? If not, I believe that name and type should just be String rather than String?, and then this gets even simpler.

offeringsLabel.text = model?.offerings
    .map { "\($0.name),\($0.type)" }
    .joined(separator: " | ")

This code is has a slight difference from your code. When model is nil, your code leaves offeringsLabel alone. My code clears the text. This raises a deeper question: why are you running this code at all when model is nil? Why is model optional? Could you either make it non-optional in the data, or should you have a guard let model = model else { return } earlier in this method?

The common cause of over-complicated Swift in my experience is the over-use of Optional. Optional is incredibly important and powerful, but you shouldn't use it unless you really mean "having no value at all here is both legitimate, and different than just 'empty'."

Shagreen answered 30/11, 2018 at 20:36 Comment(8)
i really appreciate your efforts towards community.Platinize
"Do you really treat a nil name differently than an empty name" That's a good question. Not sure what measures are to be used to answer it. Only one I can think of is if it changes yours flow. e.g. if you don't have a name, then don't instantiate this object, or don't show this label. Can you add more to that?Doti
@honey You'd have to have a flow where having no name would fail to show the label, but having an empty name would show the label. That would be a very surprising difference. A case where this actually does come up, however, is in multi-layer defaults. There's a difference between "this setting is set to empty" vs "this setting (nil) inherits its parent." And in that case, String? certainly makes sense.Shagreen
@RobNapier can you help here - stackoverflow.com/questions/55627683/…Platinize
FYI based on your answer I updated my answer here and compared the compilation time between concatenating optional and non-optional strings. concatenating optional was super slow, concatenating non-optionals seemed to take no toll at all when + was used. And so II have a question: would it be correct to say that the Apple has resolved non-optional string concatenation or there are still cases that it can effect compilation time?Doti
Also your answer is a bit mute on why usage of optionals increases compilation time. I can understand the extra null check, but that shouldn't cause 20000X more time?Doti
@Honey Things with exponential growth rates can absolutely explain 20000X more time. It's not an extra null check. It's an extra check for every combination of every possible type also possibly being Optional. It's a factorial problem. I haven't explored recently if the compiler has significantly improved; I wouldn't be shocked if it did.Shagreen
Thanks. So if I understand you correctly you're saying the difference of firstName + lastName vs (firstName ?? "") + (lastName ?? "") is 103 * 103 vs. 206 * 206. But with three adds, that would just be a difference of 128X.Doti
C
2

My suggestion is first to add a property description in the Offering object to handle name and value properly (your solution puts always a comma between name and value regardless whether name has a value or not)

var description : String? {
    let desc = [name, value].compactMap{$0}.joined(separator:",")
    return desc.isEmpty ? nil : desc
}

And rather than a loop use compactMap and joined

offeringsLabel.text = model?.offerings?.compactMap { $0.description }.joined(separator:" | ") ?? ""
Cholon answered 30/11, 2018 at 12:56 Comment(1)
Note that this returns a different value for a nil name or value than the other solution. (That could be good or bad, and really points to the fact that name and value should almost certainly not be Optional in the first place.)Shagreen
N
1

Rather than assigning a text to UILabel in each iteration and reading it again in next one, you can use Array.reduce to first get the full string

let fullString = (model?.offerings ?? []).reduce("", { string, value in
    string + " | " +  (value.name ?? "") + "," +  (value.type ?? "")
}
offeringsLabel.text = fullString

Setting text repeatedly will hamper performance because, for example, it can trigger size recalculation for dynamically sized labels

Noisome answered 30/11, 2018 at 11:48 Comment(0)
P
0

You could try the append function eg:

let valueName = value.name ?? ""
offeringsLabel.text?.append(valueName) 
Pushball answered 30/11, 2018 at 11:54 Comment(1)
That is not even valid SwiftLarcher
N
0

You should use temp variable here. Operator ?? may increase compile time dramatically if use it inside complex expressions

so you can update your code with following (Yes it's not short, but we should help to compiler)

let offerings = model?.offerings ?? []
var offeringsText = ""
for value in offerings {
    let name = value.name ?? ""
    let type = value.type ?? ""
    let valueText = " | " +  name + "," +  type
    let offeringsText = offeringsText + valueText
}
offeringsLabel.text = offeringsText

Hope this will help you!

Noleta answered 30/11, 2018 at 20:17 Comment(0)

© 2022 - 2024 — McMap. All rights reserved.