Potential performance bottleneck when using reflection.

[RabugenTom]

I'm reporting an issue I had with TCA. My app was lagging a lot and instruments showed that a bunch of the time was spent on the main thread computing debug description strings. This is caused by Mirror(reflecting:) which is used when trying to reassign local cases (EnumReflection.swift#L61).

I have huge data structs, with many containers and NSAttributedString, and generating the debug description doesn't come for free. The app is quite complex, but in the current state, ~80% of its work on the main thread is spent extracting CasePath's. I temporarily improved the situation by carped-bombing my data structs with a some protocol extending CustomReflectable and providing a very small default implementation, but this is far from optimal.

I think that this kind of call is not expected to happen on the main thread, furthermore at a high frequency rate like it happens in a reducer.

Maybe the result can be cached, or another approach to reflection can be used, with some kind of (optional) protocol or prescription over Action enums?

[mbrandonw]

Hey @RabugenTom,

The performance of case path reflection is a known issue, and it's definitely a bummer. We're not sure there is anything we can do to speed it up. Ideally Swift would provide first class support for this (like they do for key paths), which would make it much much faster.

We have found that reflection case paths work just fine as long as actions aren't being sent at a rapid pace. We're working on a large application right now and the reflection hasn't been an issue yet.

However, if you do have some actions that are being sent in great volume, then you may need to write your case paths by hand rather than relying on the reflection. There's a discussion in this issue on TCA discussing this very topic:

pointfreeco/swift-composable-architecture#327

Let us know if that helps at all.

Also, I do not believe your performance problems are coming from usage of .debug() (or at least it shouldn't be!). By default .debug() uses a synchronous background queue to do printing, and so hopefully it shouldn't slow down your application:

https://github.com/pointfreeco/swift-composable-architecture/blob/c1f88bd8608e8a26fd61ab46c9f11a63b0f4023d/Sources/ComposableArchitecture/Debugging/ReducerDebugging.swift#L158-L161

If your state is really large then I suppose there's a chance that printing the state is taxing your CPU and causing everything to slow down. However, none of that would show up as work on the main thread in instruments. Further, I don't think the code in EnumReflection.swift is executed at all for the .debug() operator. It's only in the case path reflection code.

[RabugenTom]

Thanks Brandon, I'll look at the hand-crafted case paths. I wasn't aware of them. I'm already using Sourcery to generate a bunch of things in my app, so I guess can write a template to generate case paths automatically. I'll report if it solves the issue when restoring default Mirror's , description's, and debugDescription's.

When I'm talking high rate, I'm considering a some kind of evolved macOS word processor using SwiftUI with two sidebars and many features (like between 5 and 10 "iOS screens" visible at the same time). There is not much I can do to lower actions traffic below the point I'm at, but as I said, the apps runs fine as soon as I'm able to mute my types' (debug)Description's, as ~80% of the time spent on the main thread was lost because of this during CasePath resolution.

Also, I'm not using .debug() at any place. In Instruments, the debug strings appear to be requested by extractHelp:

…
 closure #1 in static CasePath.case(_:)
  extract<A, B>(case:from:)
   extractHelp #1 <A, B>(from:) in extract<A, B>(case:from:)
    _print_unlocked<A, B>(_:_:)
     _adHocPrint_unlocked<A, B>(_:_:_:isDebugPrint:)
      _debugPrint_unlocked<A, B>(_:_:)
       _adHocPrint_unlocked<A, B>(_:_:_:isDebugPrint:)
        _debugPrint_unlocked<A, B>(_:_:)
         _adHocPrint_unlocked<A, B>(_:_:_:isDebugPrint:)
          _debugPrint_unlocked<A, B>(_:_:)
           _adHocPrint_unlocked<A, B>(_:_:_:isDebugPrint:)
            _debugPrint_unlocked<A, B>(_:_:)
             _adHocPrint_unlocked<A, B>(_:_:_:isDebugPrint:)
              _debugPrint_unlocked<A, B>(_:_:)
               _adHocPrint_unlocked<A, B>(_:_:_:isDebugPrint:)
                _debugPrint_unlocked<A, B>(_:_:)
                 Array.description.getter
                  specialized _debugPrint<A>(_:separator:terminator:to:)
                   _debugPrint_unlocked<A, B>(_:_:)
                    _adHocPrint_unlocked<A, B>(_:_:_:isDebugPrint:)
                     _debugPrint_unlocked<A, B>(_:_:)
                      _adHocPrint_unlocked<A, B>(_:_:_:isDebugPrint:)
                       _debugPrint_unlocked<A, B>(_:_:)
                        Optional.debugDescription.getter
                         specialized _debugPrint<A>(_:separator:terminator:to:)
                          _debugPrint_unlocked<A, B>(_:_:)
                           _adHocPrint_unlocked<A, B>(_:_:_:isDebugPrint:)
                            _debugPrint_unlocked<A, B>(_:_:)
                             _adHocPrint_unlocked<A, B>(_:_:_:isDebugPrint:)
                              _debugPrint_unlocked<A, B>(_:_:)
                               Optional.debugDescription.getter
                                specialized _debugPrint<A>(_:separator:terminator:to:)
                                 _debugPrint_unlocked<A, B>(_:_:)
                                  _adHocPrint_unlocked<A, B>(_:_:_:isDebugPrint:)
                                   _debugPrint_unlocked<A, B>(_:_:)
                                    _adHocPrint_unlocked<A, B>(_:_:_:isDebugPrint:)
                                     _debugPrint_unlocked<A, B>(_:_:)
                                     _IteratorBox.next()…

Upon closer inspection, it seems that the issue may not arise from Mirror instantiation at L61, but from String(describing:) at L63 instead, as _print_unlocked suggests (see Swift's Mirror.swift). Maybe there is an alternative way to retrieve the path there, but I'm still very new to CasePath's codebase so I need to dig more into this.

[RabugenTom]

I've written some Sourcery template to generate all my case paths. I confirm that using these explicit definitions resolves my performance issues.

I'm attaching the template here in any case someone wants to make use of it. You can also include it in swift-case-path if you want. You simply have to declare an AutoCasePath marker protocol and mark your Root enums with it. It generates case paths as static members of CasePath<Root, LocalAction> for actions of the kind .something(LocalAction) whenever LocalAction is AutoCasePath itself. It also works for indexed actions in the form .something(Key, LocalAction).

import CasePaths

// This template generates CasePath for enumerations implementing the `AutoCasePath` marker protocol.
// Declare `protocol AutoCasePath {}` in your code and mark enumerations that should participate to
// case path generation. For example:
//
//   protocol AutoCasePath {}
//
//   enum AppAction: AutoCasePath {
//     case profile(ProfileAction)
//   }
//   enum ProfileAction: AutoCasePath {
//     case buttonTapped
//   }

{% for type in types.enums where type.implements.AutoCasePath %}
{% for member in type.cases where member.hasAssociatedValue  %}
{% if member.associatedValues.count == 1 and member.associatedValues[0].type.implements.AutoCasePath %}
// CasePath for {{ type.name }}.{{ member.name }}({{ member.associatedValues[0].typeName }})
extension CasePath where Root == {{ type.name }}, Value == {{ member.associatedValues[0].typeName }} {
  {{ type.accessLevel }} static let {{ member.name }} = Self(
    embed: {{ type.name }}.{{ member.name }},
    extract: {
      guard case let .{{ member.name }}(action) = $0 else { return nil }
      return action
    }
  )
}

{% elif member.associatedValues.count == 2 and member.associatedValues[1].type.implements.AutoCasePath %}
// CasePath for {{ type.name }}.{{ member.name }}({{ member.associatedValues[0].typeName }}, {{ member.associatedValues[1].typeName }})
extension CasePath where Root == {{ type.name }}, Value == ({{ member.associatedValues[0].typeName }}, {{ member.associatedValues[1].typeName }}) {
  {{ type.accessLevel }} static let {{ member.name }} = Self(
    embed: {{ type.name }}.{{ member.name }},
    extract: {
      guard case let .{{ member.name }}(key, action) = $0 else { return nil }
      return (key, action)
    }
  )
}

{% endif %}
{% endfor %}
{% endfor %}

[maximkrouk]

Extracted the template into a gist

[maximkrouk]

Do you know how to set the output to {{ file.path }}{{ file.name }}.AutoCasePath.swift?
I have a modular architecture and output to one location is useless for me ;-;
I spent about 2 hours trying to find a way and bought sourcery pro, but still can't get it done 🥲

[tgrapperon]

The destination path is set when calling Sourcery if I'm not mistaken. Assuming you're using SPM, this is not trivial as we don't have build scripts (yet). You will need to write a utility script which will run Sourcery on each of your packages. You can use a marker file in your module's root to indicate your script if it should run Sourcery there (an possibly pass some options if the file you're using to mark eligible modules is .sourcery.yml). If there is no such file, you script doesn't run Sourcery for this module.

I would also rewrite the script to work on each case with associated value, whatever the associated value is (I would get rid of the marker protocol at the same time and generate CasePaths for every case with associated value of every enum). Because of modularization, each run of Sourcery will only see the local module, and you will miss AutoCasePath conformance declared in other modules, and the generated code will miss some cases. Similarly, you will need to import a common AutoCasePath marker protocol (or make it internal for each module). I don't remember if Sourcery is able to work if it doesn't see the protocol declaration. That's why I would try get rid of the marker protocol. I think removing all filtering related to type.implements.AutoCasePath in the script does the trick.

These advices are limited by my knowledge of Sourcery. I would be better if Sourcery would be able to generate one file per file where it encounters the enums, but I don't think that's possible. This template was used in an app with a few old-style frameworks, not a hyper-modularized architecture like isowords.

As you need to write a script, you can also forgo Sourcery and use swift-syntax to spot your enums and generate case-paths in separate files. If I'm not mistaken, all the info you need to generate the code is contained in the enum's declaration, so it should be achievable. You parse all Swift files in your repo, extract enum's declarations (which are quite formatted in general, so you can even use your own parser if swift-syntax is too heavy). You then generate the case-paths and save a new file along the file you've just processed. I think it can be done in a few hours, and you'll definitely be less frustrated than trying to make Sourcery work in this simple case. That's how I'll do.

If it's for a proof of concept, you can also run Sourcery in your whole repository and copy-paste declarations along the enums to quickly check if it really improves your performance issues, and if you should bother automating the process with a custom script.

[tgrapperon]

This modified template can be run manually with SourceryPro's Xcode extension in each file you declare an action. You don't need the AutoCasePath protocol anymore.

{% for type in types.enums %}
{% for member in type.cases where member.hasAssociatedValue %}
{% if member.associatedValues.count == 1 %}
// CasePath for {{ type.name }}.{{ member.name }}({{ member.associatedValues[0].typeName }})
extension CasePath where Root == {{ type.name }}, Value == {{ member.associatedValues[0].typeName }} {
  {{ type.accessLevel }} static let {{ member.name }} = Self(
    embed: {{ type.name }}.{{ member.name }},
    extract: {
      guard case let .{{ member.name }}(action) = $0 else { return nil }
      return action
    }
  )
}

{% elif member.associatedValues.count == 2 %}
// CasePath for {{ type.name }}.{{ member.name }}({{ member.associatedValues[0].typeName }}, {{ member.associatedValues[1].typeName }})
extension CasePath where Root == {{ type.name }}, Value == ({{ member.associatedValues[0].typeName }}, {{ member.associatedValues[1].typeName }}) {
  {{ type.accessLevel }} static let {{ member.name }} = Self(
    embed: {{ type.name }}.{{ member.name }},
    extract: {
      guard case let .{{ member.name }}(key, action) = $0 else { return nil }
      return (key, action)
    }
  )
}

{% endif %}
{% endfor %}
{% endfor %}

[maximkrouk]

Well, sourcery has {{ type.path }}, and it's probably what I've been searching for, but I already imported swift-format to spmgen and implemented CasePaths generation for simple enums, I'm a bit tired, so I'll continue a bit later (to support nested and generic enums) and mention the result here and in pointfreeco/swift-composable-architecture#585 (comment)

[mbrandonw]

Hey @RabugenTom, thanks for the info! That is very interesting.

We'd love to figure out how to make the reflection API faster. If you figure anything out please let us know.

I'm pretty surprised that stack trace is so deep for you. Wonder what String(describing:) is doing. Is there any chance you could share your enums for us to check out?

Thanks!

[RabugenTom]

Hey @mbrandonw, I've checked again and the issue is not where I thought it was (twice…). I've implemented some custom debug description in one of my data type and put a breakpoint to see where it was called. I should have done this from the start instead of making you loose your time.

The issue arises from L70, in ["\(root)"].
It is particularly visible when root is some collection. I don't know yet how to resolve this yet since there is nothing semantically wrong with the algorithm. One can maybe think about using "\(root.hashValue)" instead of "\(root)" whenever available. This doesn't seem to cause any issue and tests are still passing.
Or maybe some generalised Parser can also be used to extract the Value's ;-).

About my stack trace being deep, this is not so surprising given the architecture of my app (and this doesn't take so long to dive into it when we are not in the special cases presenting the issue). It's a macOS app which allows to write songs with chords and tablatures with an enhanced text editor. What you see in the stack trace up to Array.description.getter seems related to the [Root] -> [Editing] -> [SideBar -> SideBarModule -> SideBarModuleContent -> SideBarModuleContent.View] chain of features/actions I'm using. What happens after the Array.description.getter would then be related the issue I was having, where a collection description was being generated. I'm deducing this because I have an action with such problematic associated value in SideBarModuleContent.View.Actions. I'll extract the enums when the app is ready. Since they are now marked with my AutoCasePath protocol, I can extract them all in one place with another Sourcery template.

[stephencelis]

@RabugenTom Not sure if you ever revisited this, but I'm going to move to a discussion. Maybe more folks will have ideas they can share there!

[RabugenTom]

I think you fixed the underlying issue (when enums had only one case (see #18)).
I closed the PR but forgot to close the issue.

[stephencelis]

I wasn't sure if that fixed everything 😄 there are definitely more performance gains we think could be made, though!

[RabugenTom]

One way to do it would be to manage to cache the results, but I don't know if the overhead (in complexity and at runtime) is worth it. As long as this doesn't ship with Swift itself, I guess generating them with Sourcery or another codegen tool is quicker/safer if reflection is not fast enough.

[mbrandonw]

@RabugenTom we’ve recently merged a bunch of performance improvements to this library. I’d be curious if you see any changes in your instruments.

[RabugenTom]

Thanks @mbrandonw! In my case, it was mainly caused by a bug that was fixed shortly after (when one enum early in the path had only one case). I'm super exited with recent developments though!