Closed mayoff closed 3 years ago
The new version (7cab867) fixes the existential bug and passes all the tests (including some new ones).
Code coverage of EnumReflection.swift
indicates the tests cover everything except an “impossible” case where some metadata is missing, and a mangled type name encoding that I don't know how to trigger.
It runs faster but slower in the failure case.
Main branch (90e4e67e):
name time std iterations
---------------------------------------------------
Success.Manual 34.000 ns ± 146.77 % 1000000
Success.Reflection 151.000 ns ± 121.01 % 1000000
Failure.Manual 35.000 ns ± 231.80 % 1000000
Failure.Reflection 65.000 ns ± 191.00 % 1000000
New version:
name time std iterations
---------------------------------------------------
Success.Manual 33.000 ns ± 226.73 % 1000000
Success.Reflection 128.000 ns ± 85.30 % 1000000
Failure.Manual 37.000 ns ± 149.48 % 1000000
Failure.Reflection 73.000 ns ± 140.36 % 1000000
Surprisingly, unwrapping just the cached tag first to reject non-matching values (instead of unwrapping the whole cached extractor) consistently saves a few nanoseconds in the Failure.Reflection benchmark.
name time std iterations
---------------------------------------------------
Success.Manual 34.000 ns ± 261.66 % 1000000
Success.Reflection 135.000 ns ± 116.75 % 1000000
Failure.Manual 36.000 ns ± 221.68 % 1000000
Failure.Reflection 66.000 ns ± 174.90 % 1000000
I think I'm done dinking this.
I've tried a few variations to knock nanoseconds off the Failure.Reflection benchmark without success, like storing -1 for cachedTag instead of using an optional, but was unable to make it consistently faster. I suspect it's consistently slower than the main branch by ~3 nanos because the closure captures two words (the cached tag and the strategy) instead of just one (the cached tag).
I thought the whole private helpers section of the file was kind of jumbled so I reorganized it. The code is all the same, but hopefully it's in a more logical order now so it should be easier to read from top to bottom.
Really awesome work @mayoff! Thanks heaps for the informative comments and commits :) it's looking pretty close to optimal.
This branch is not ready for merging. I'm creating this pull request only to make it available for discussion.
I found example uses of
swift_getTypeByMangledNameInContext
in the Reflect package and have made some progress toward using it to replace the use ofMirror
.In its current state, this branch has several failing tests, all related to associated value labels. I've outlined what's wrong and what needs to be done in the commit message and code comments.