Reimplement _forEachField from the stdlib and use it (aka perverse runtime black magic)#397
Draft
kaascevich wants to merge 6 commits intomoreSwift:mainfrom
Draft
Reimplement _forEachField from the stdlib and use it (aka perverse runtime black magic)#397kaascevich wants to merge 6 commits intomoreSwift:mainfrom
_forEachField from the stdlib and use it (aka perverse runtime black magic)#397kaascevich wants to merge 6 commits intomoreSwift:mainfrom
Conversation
_forEachField from the stdlib and use it_forEachField from the stdlib and use it (aka perverse runtime black magic)
Collaborator
|
I'll leave this as a draft until we've done some benchmarking, because I probably won't merge the PR unless we have a compelling reason, due to the risks with using SPI runtime functions that may change at any time (afaict). |
Contributor
Author
|
That is fair and I absolutely agree with that. |
Collaborator
|
Alternatively, if we can get a guarantee that these runtime APIs are safe to use, then I would be happy to merge even if the performance gains are negligible, because it's a much nicer approach than my current Mirror-based performance hack. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
There's a function called
_forEachField(of:options:body:)in the standard library, calls directly into the runtime to retrieve the names, types, and (importantly for us) offsets of a type's stored properties. Since it doesn't create anyMirrorinstances, it's (presumably) more performant than iterating overMirror.children, and it's unaffected byCustomReflectableconformances.This function would be perfect for updating dynamic properties, but there's a catch:
_forEachField(of:options:body:)is annotated with@_spi(Reflection), effectively making it inaccessible in release toolchains. So what I did was copy the stdlib's implementation into SwiftCrossUI, then shrink it down to what we actually need. In the end, we only required three runtime functions:(
_FieldReflectionMetadatacomes from theSwiftShimsmodule, which for reasons beyond my mortal comprehension is completely accessible and can be imported like any other module.)Here's the Swift Forums thread @stackotter created discussing whether these functions are safe for us to use long-term -- we never got an official answer from the Swift team, but the conclusion reached by @tera was:
Of course, benchmarking this to make sure that it's actually meaningfully faster than the current implementation will be a necessity.