stream-chat-swiftui icon indicating copy to clipboard operation
stream-chat-swiftui copied to clipboard

Large compile time increase after adding new functions to ChatViewFactory

Open AndrewSB opened this issue 1 year ago • 7 comments

What did you do?

added two new functions to our ChatViewFactory

What did you expect to happen?

to have a linear increase in compile times

What happened instead?

compile times for our target went from about ~20s to ~400s

Additional context

@martinmitrevski we dealt with this a while ago, our approach of using typealiases, and keeping the function bodies simple didn't seem to work for these two new functions. if we make comment out these 2 functions, the compile times go back down. i don't really understand why. have you got any ideas?

Screen Shot 2022-10-03 at 6 34 17 PM

AndrewSB avatar Oct 04 '22 19:10 AndrewSB

Hey @AndrewSB,

First question - have you already migrated to Xcode 14? I've heard on some WWDC videos they might huge improvements to the compile times in Swift 5.7.

If you're still getting slow build times in Xcode 14, there are several things we could try:

  • maybe we can provide default associated types in the SDK: https://twitter.com/schwa/status/1577522396419960832/photo/1. If you think this can help, we can prepare a branch for you to test.
  • might be good idea to go through the customizations and see if some of them can be replaced by configuration that we can expose. Goal would be to reduce the number of customizations while trying to use more configuration.
  • this post also has some nice tips: https://medium.com/atlas/optimizing-build-times-in-swift-4-dc493b1cc5f5. Might be worth to skim through it and try some things.

Hope this helps and looking forward to your feedback.

martinmitrevski avatar Oct 05 '22 07:10 martinmitrevski

yup, we had our team update a few weeks ago

reading through your links now,

  1. i think switching to some default associated types could be useful, but i don't know if it'll actually help in most cases (if i understand it correctly). in every case where we override, the return that you define and the what we return in our overridden function will always be different, so it shouldn't save any time, unless you think just having a default type will speed up type inference
  2. you know better than me which ones are good categories for configuration instead of customization. i've also been using a pattern where i pass a Type which has static functions that perform work, i'll share a screenshot below. i don't know if i like it yet, but it's been interesting
  3. yeah we're already doing most of that, this ballooning compile time here is just the type inference time for the ChatViewFactory
public protocol RemoteLoggingType {
    static func setUserID(_ userID: String?)

    static func info(_ message: String)
    static func error(_ message: String)
    static func fault(_ message: String)
    static func debug(_ message: String)
    static func verbose(_ message: String)

    static func recordError(error: Error)
}

// and then at setup time, we set
Log.remoteLogger = TypeThatConformsToProtocol.self
// since all the functions are static, we never need to initialize the type

AndrewSB avatar Oct 07 '22 00:10 AndrewSB

hey @AndrewSB, thanks for the update.

  1. I will prepare a branch, so at least we can try.
  2. If it doesn't solve it, I'll go through your integration and see if some of your usage can be reverted to the default SDK views. Will get back to you on this early next week.

martinmitrevski avatar Oct 07 '22 15:10 martinmitrevski

Hey @AndrewSB,

I've created a branch called default-values-types for the associated types with default values. Please check if it improves the build times, or we would need a different approach.

martinmitrevski avatar Oct 10 '22 13:10 martinmitrevski

we gave it a shot, seems ineffective. we've #if DEBUGd out those 2 functions from above

image

AndrewSB avatar Oct 11 '22 00:10 AndrewSB

Hey @AndrewSB, I've had a look at your view factory. Here are some additional suggestions:

  • could you try to add typealiases to the views that are missing them, e.g. makeMessageComposerViewType and few more.
  • could you move the ones that contain if statements into separate views (e.g. with ZStack or Group) and add typealiases for those?
  • for some views, like ReclipMessageTextView I think you can move to our view, since from what I can see, only the paddings are different, and we expose those in the intializer of our own view.

martinmitrevski avatar Oct 11 '22 16:10 martinmitrevski

will investigate those suggestions and get back to you, thanks @martinmitrevski

AndrewSB avatar Oct 11 '22 18:10 AndrewSB

Closing this because of inactivity. If you still need our support on this, feel free to reopen it.

martinmitrevski avatar Oct 30 '22 22:10 martinmitrevski