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

mfaani
  • 33,269
  • 19
  • 164
  • 293
Amit
  • 556
  • 1
  • 7
  • 24
  • 2
    Mostly it's `??` — it has some problems with type inference checks during build. For reference, check [here](https://medium.com/@RobertGummesson/regarding-swift-build-time-optimizations-fc92cdd91e31). – user28434'mstep Nov 30 '18 at 11:46
  • @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]?` – Amit Nov 30 '18 at 12:06
  • 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. – user28434'mstep Nov 30 '18 at 12:13
  • 1
    Use guards so you don't have all those optionals. Then it will perform a lot better. – Murf Nov 30 '18 at 12:15

5 Answers5

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'."

Rob Napier
  • 286,113
  • 34
  • 456
  • 610
  • i really appreciate your efforts towards community. – Amit Dec 04 '18 at 07:27
  • "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? – mfaani Feb 05 '19 at 19:44
  • @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. – Rob Napier Feb 05 '19 at 21:21
  • @RobNapier can you help here - https://stackoverflow.com/questions/55627683/calendar-current-datebysettinghour-takes-so-long-to-compile – Amit Apr 11 '19 at 08:21
  • FYI based on your answer I updated my answer [here](https://stackoverflow.com/questions/29707622/swift-compiler-error-expression-too-complex-on-a-string-concatenation/39514629#39514629) 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? – mfaani Feb 05 '20 at 20:23
  • 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? – mfaani Feb 05 '20 at 20:56
  • @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. – Rob Napier Feb 05 '20 at 21:06
  • 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. – mfaani Feb 05 '20 at 21:18
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:" | ") ?? ""
vadian
  • 274,689
  • 30
  • 353
  • 361
  • 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.) – Rob Napier Nov 30 '18 at 20:40
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

mag_zbc
  • 6,801
  • 14
  • 40
  • 62
0

You could try the append function eg:

let valueName = value.name ?? ""
offeringsLabel.text?.append(valueName)
alitosuner
  • 984
  • 1
  • 10
  • 15
Merl
  • 305
  • 2
  • 9
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!

Nick Rybalko
  • 202
  • 2
  • 5