pointfreeco / swift-dependencies

A dependency management library inspired by SwiftUI's "environment."
https://www.pointfree.co
MIT License
1.56k stars 125 forks source link

Profile builds crash on `continuousClock` dependency #64

Closed jaroslavas closed 6 months ago

jaroslavas commented 1 year ago

Description

When profiling the app that depends on continuousClock, it crashes. Confirmed both on my pp and on CaseStudies (SwiftUI) example app from swift-composable-architecture.

Checklist

Expected behavior

It shouldn't crash :)

Actual behavior

App crashes.

Steps to reproduce

  1. Check out swift-composable-architecture repo
  2. Open ComposableArchitecture.xcworkspace and select CaseStudies (SwiftUI) scheme
  3. Product -> Profile
  4. Select Leaks
  5. App crashes.

Crash log points to 21 SwiftUICaseStudies 0x105ceaa06 Root.init() + 12 (00-Core.swift:63) [inlined]. In my app, it also points to the line Dependency(\.continuousClock) var clock, so looks like there is a problem with continuousClock dependency itself.

Dependencies version information

0.2.0

Destination operating system

iOS 16

Xcode version information

14.2 (14C18)

Swift Compiler version information

No response

mbrandonw commented 1 year ago

Hi @jaroslavas, I'm not sure the crash has anything to do with the continuous clock. I can comment out the clock in Core.swift, as well as all features using clocks, and it still doesn't seem to run in instruments.

jaroslavas commented 1 year ago

That's weird. On my app, when I commented out the clock and all the code that uses it I was able to profile the app.

lightandshadow68 commented 1 year ago

I've run into the same problem. Looks like it's a name demangling issue.

0x1b17e3fa8 pthread_kill + 8 1 libsystem_pthread.dylib 0x1b183812c pthread_kill + 256 2 libsystem_c.dylib 0x18012873c abort + 124 3 libswiftCore.dylib 0x18bffdf90 demangleFatal(unsigned int, char const, char) + 128 4 libswiftCore.dylib 0x18bffdf10 swift::Demangle::runtime::fatal(unsigned int, char const, ...) + 32 5 libswiftCore.dylib 0x18bfc58b4 swift::Demangle::__runtime::failAssert(char const, unsigned int, swift::Demangle::runtime::Node, char const) + 104 6 libswiftCore.dylib 0x18bfc5f2c swift::Demangle::runtime::Node::addChild(swift::Demangle::runtime::Node*, swift::Demangle::runtime::NodeFactory&) + 584 7 libswiftCore.dylib 0x18bf9aaf4 copyGenericClassObjCName(swift::TargetClassMetadata<swift::InProcess, swift::TargetAnyClassMetadataObjCInterop>) + 228 8 libobjc.A.dylib 0x18003a4bc objc_class::installMangledNameForLazilyNamedClass() + 100 9 libobjc.A.dylib 0x1800401f8 objc_class::demangledName(bool) + 72 10 liboainject.dylib 0x105ff2ad4 0x105fec000 + 27348 11 libobjc.A.dylib 0x180033f5c initializeNonMetaClass + 676 12 libobjc.A.dylib 0x180035700 initializeAndMaybeRelock(objc_class, objc_object, locker_mixin<lockdebug::lock_mixin>&, bool) + 192 13 libobjc.A.dylib 0x1800430cc lookUpImpOrForward + 612 14 libobjc.A.dylib 0x18002f2e0 _objc_msgSend_uncached + 64 15 libswiftCore.dylib 0x18bfc368c swift_instantiateObjCClass + 24 16 libswiftCore.dylib 0x18bf91ef8 _swift_initClassMetadataImpl(swift::TargetClassMetadata<swift::InProcess, swift::TargetAnyClassMetadataObjCInterop>, swift::ClassLayoutFlags, unsigned long, swift::TypeLayout const const, unsigned long, bool) + 2944 17 libswiftCore.dylib 0x18bf623b4 type metadata completion function for WritableKeyPath + 28 18 libswiftCore.dylib 0x18bf8e2b0 _swift_getGenericMetadata(swift::MetadataRequest, void const const, swift::TargetTypeContextDescriptor const) + 1688 19 libswiftCore.dylib 0x18bf707b8 __swift_instantiateGenericMetadata + 28 20 libswiftCore.dylib 0x18bdbfacc _swift_getKeyPath(pattern:arguments:) + 128 21 Capture-Dev 0x102d1ae8c AListFeature.init() + 1156 (AListFeature.swift:134)

This points to @Dependency(\.continuousClock) var clock

Removing this dependency allowed it to run in Instruments.

mbrandonw commented 1 year ago

Hi all, OK I have definitely been able to reproduce this, and definitely see that it is the \.continuousClock dependency causing the problem. I removed all clocks from the case studies and it no longer happened.

Further, it seems related to existentials. The continuousClock dependency is currently modeled as any Clock<Duration>, which gives us nice ergonomics, but unfortunately leads to crashes like this. If we swap out that dependency for a concrete AnyClock type erased wrapper (which ships with our Clocks library to work around other existential bugs), then profiling will work:

import Clocks
private enum AnyContinuousClockKey: DependencyKey {
  static let liveValue = AnyClock(ContinuousClock())
}
extension DependencyValues {
  var anyContinuousClock: AnyClock<Duration> {
    get {
      self[AnyContinuousClockKey.self]
    }
  }
}
@Dependency(\.anyContinuousClock) var clock

This is a pretty big bummer. If we go in and change \.continuousClock to use AnyClock instead of any Clock it will be a huge breaking change for people. But also leaving it as-is is quite problematic. I'm not sure what we should do.

At the very least you can make use of this \.anyContinuousClock in your own projects for the time being to get around the problem.

Incidentally we also stumbled upon yet another existential bug just this morning: https://github.com/apple/swift/issues/64974 😭

mbrandonw commented 1 year ago

I filed a feedback, and suggest anyone who has run into this issue also dupe it: https://gist.github.com/mbrandonw/8ea305e8b7ec169b3e04990e749a1236#file-fb12101499-md

lightandshadow68 commented 1 year ago

That did the trick.

I was attempting to profile with Instruments, as I've run into an issue where using a NavigationLink with a destination View that has a @Binding that results in an infinite loop. . _print() indicates a re-render due to all of the bindings changing in the presenting view. It too results in the growth of memory. Init is not being called on any of the model structs. Nor is the initializer called on either the SwiftUIView that presents the link or the destination View. It quickly jumps to 1G+, and keeps going. Wondering if this is related.

lightandshadow68 commented 1 year ago

The issue was caused by using NavigationStack instead of NavigationView.

mbrandonw commented 1 year ago

Wow, good find. I can reproduce that too. That is a serious bug in NavigationStack!

lightandshadow68 commented 1 year ago

Switched to using NavigationLink(value:label) and .navigationDestination(for:destination) with NavigationStack as part of the iOS 16 navigation API, but this didn't seem to help. What's odd is that I can even have default constant bindings for parameters and _print() still says the bindings changed. But self does not change. Nor does the parent view initializer get called, etc. Seems to be looping somewhere in SwiftUI as I've even set my custom bindings to do nothing in the set block and _print() still says they changed, causing a re-render.

Now that I think about it, should binding changes cause re-renders? Seems like that should be limited to @State properties.

SubjectsFiltersSummaryView: _filterSettings, _hideAbsent, _hideCheckedIn, _hideDiscared changed. SubjectsFiltersSummaryView: _filterSettings, _hideAbsent, _hideCheckedIn, _hideDiscared changed. SubjectsFiltersSummaryView: _filterSettings, _hideAbsent, _hideCheckedIn, _hideDiscared changed. [...]

Switching back, those same properties are listed as changing when using NavigationView, but it doesn't loop. Seems like this is specific to navigation. As if it's being re-rendered due to navigation happening, which is not a property in the sense that state and binding properties are?

But, yes, this seems like a rather common use case. I'm reusing a filtering UI between features and didn't want it to send parent feature specific TCA actions.

Here's another interesting one. I think the programatic push from the content column issue is probably known. However, the duplicate .navigationDestination with NavigationSplitView issue seems like a conceptual / documentation gap : https://developer.apple.com/forums/thread/727778

iampatbrown commented 11 months ago

It looks like the view's equatability may be contributing to this issue.

Added some possible work arounds to apple/swift#64974. Unsure if useful.

mbrandonw commented 6 months ago

I am going to convert this to a discussion as it's not a bug with the library but rather some confluence of Swift/SwiftUI/existentials.