Avoid creating multiple Mirror instances for the same value.

[yimajo]

Avoid creating multiple Mirror instances for the same value.

Motivation:

Initializing __Expression.Value currently creates an unnecessary Mirror instance. This does not increase the algorithmic complexity, but the initializer call chain makes it difficult to identify where the extra initialization occurs.

Specifically, the internal init(reflecting:) calls the private init(_reflecting:label:seenObjects:depth:options:), which in turn calls the internal init(describing:).

Within this call chain, init(describing:) creates a Mirror. However, the private init(_reflecting:label:seenObjects:depth:options:) cannot reuse it and creates another Mirror for the same subject.

The root cause appears to be that init(describing:) is also used to initialize specific properties, making it tempting to call it from the private init(_reflecting:label:seenObjects:depth:options:).

Modifications:

Changed the private init(_reflecting:label:seenObjects:depth:options:) to initialize description, debugDescription, and typeInfo without calling the internal init(describing:). The private initializer now creates only one Mirror instance for the subject.

Added _describingProperties(of:) to avoid repeating the expressions used to initialize these properties in both initializers.

Checklist:

• Code and documentation should follow the style of the Style Guide.
• If public symbols are renamed or modified, DocC references should be updated.

[grynspan]

Thank you for the PR. This change does not appear to be substantive nor does it appear to resolve the issue you describe. As such, I am closing it, but we can reopen it if I've misunderstood or if you'd like to revise the PR.

[grynspan]

It is generally considered good form to reuse initializers this way; perhaps a better approach is to have a private third "designated" initializer that takes a Mirror as an argument? (Note that this entire type is unlikely to be preserved in Embedded Swift anyway, so I wouldn't worry about its lack of Mirror right now.)

[yimajo]

@grynspan Thank you for the comment.

The issue is shown below: the main branch creates two Mirror instances for the same subject.

   private init(
      _reflecting subject: Any,
      label: String?,
      seenObjects: inout [ObjectIdentifier: AnyObject],
      depth: Int,
      options: Configuration.ValueReflectionOptions
    ) {
      if depth >= options.maximumChildDepth {
        self = Self(describing: subject)
        isTruncated = true
        return
      }

      self.init(describing: subject) //❗️: Creates a Mirror inside init(describing:).
      self.label = label

#if !hasFeature(Embedded)
      let mirror = Mirror(reflectingForTest: subject) // ❗️: Creates another Mirror for the same subject.

This PR resolves this issue.

[grynspan]

Apologies, I misread the change.

Can you elaborate on the actual performance impact here?

[grynspan]

@yimajo Ping!

[yimajo]

I measured the execution time before and after this change using a focused microbenchmark in the default debug configuration.
I used the following Swift Testing test:

struct PR1814PerformanceTests {
  @Test
  func valueReflectionPerformance() {
    let iterationCount = 10_000
    let value: [Int?] = [0]

    let duration = ContinuousClock().measure {
      for _ in 0..<iterationCount {
        _ = Expression.Value(reflecting: value)
      }
    }

    print(
      "PR1814_BENCHMARK input=[Int?] "
        + "iterations=\(iterationCount) "
        + "duration=\(duration)"
    )
  }
}

I ran the before and after versions three times each, alternating between them. The values below are the medians of the three runs and represent the total duration of 10,000 calls to Expression.Value(reflecting:).

The results are shown in the table below.

Before (849a604)	After (375090d)	Change
146.377 ms	117.073 ms	20.02% reduction

Configuration: default debug build (-Onone), Apple M1 Max, Swift 6.3.1.

Finally, as stated in the PR description, performance optimization is not the purpose of this PR, and the algorithmic complexity remains unchanged. Delegating to init(describing:) obscures the fact that it initializes a Mirror before Expression.Value._reflecting initializes another Mirror for the same value. The purpose of this PR is to separate initialization of the shared properties from initialization of the Mirror, so the code accurately communicates the work being performed.

[grynspan]

Finally, as stated in the PR description, performance optimization is not the purpose of this PR, and the algorithmic complexity remains unchanged. Delegating to init(describing:) obscures the fact that it initializes a Mirror before Expression.Value._reflecting initializes another Mirror for the same value. The purpose of this PR is to separate initialization of the shared properties from initialization of the Mirror, so the code accurately communicates the work being performed.

If your intent is not to improve performance, then (not to be rude) "so what?" Initializer delegation is a very common pattern in Swift, and if a temporary value has to be created twice (and used in different ways, I might emphasize) that's not necessarily a problem in isolation.