-
Notifications
You must be signed in to change notification settings - Fork 522
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[net6/Xamarin.iOS] NSObject.ConformsToProtocol
is very costly and invoked very often
#14065
Comments
I do not think this would be a specific issue to net6 - but I might have missed some changes so take this with a grain of salt.
That's Apple's ObjC code calling [1] so there's
Caching would help some cases but it would help in every cases (at least versus its cost). I doubt it's a good thing to implement caching inside the platform assemblies. Still you could cache the specific cases that affects you.
Or do your own hardcoded logic to return |
Thanks for the detailed update! Indeed, I do know which types are queried there, and caching should be simple enough for sure. I wonder, is this a lookup that could be statically generated, or is the pointer passed opaque enough to be useless for caching? |
It is, in some cases. However if your app/config is allowing dynamic registration [1] then the querying happens (even if a single See [1] and that could be what differ from legacy and net6 profiles in your case. |
This is certainly an area where we can optimize the code, but we'd probably need a real-world sample of where it's a problem before spending time trying to fix a particular piece of code that may or may not be the actual problem. |
It's actually very curious, and likely something that started happening with iOS 15 (seemingly in relation to Here's a snippet from what I see in instruments: This happens with very large UIView trees. |
It might not be the exact same case but inside Dope for The static registrar is not enabled by default for [1] that subviews removal is somewhat (maybe not everywhere yet) optimized for macOS (using an API not available on iOS) so it could be worse on iOS (at least when the static registrar was not enabled). |
With a cache around Here's a newer screenshot of the previous data, this time focusing on
With the cache enabled... well it's 0s (no data) out of 4.82 seconds for Now the situation for macOS (numbers above) is a bit different than iOS since removing the dynamic registrar is not very common (beside not being documented) and my attempt for net6 crashed (still need to look why 👀). This makes calls to the reflection parts of @Therzok does this slowdown also shows up inside VSfM ? BenchmarkI made a benchmark to compare
However it's not clear it's being asked about protocols (any or as often) as a real, displayed view would 🧂. I have not adapted the cache to NumbersXamarin.Mac Legacy
[1] that's within the noise of the test, sporadic GC pauses net6.0-macos
ConclusionIMO that's impressive enough to add it (at least inside Uno's FWIW the cache related code could be added under a if (Runtime.DynamicRegistrationSupported) {
} so the linker/trimmer would remove it whenever the dynamic registrar is removed, making it a no-cost for most iOS apps. Finally it's also worth nothing that the calls to |
Details: dotnet/macios#14065 (comment) TODO * Get numbers for iOS. Since, by default, the dynamic registrar is removed on release builds this won't have a big impact (but it should not regress performance either) * See if Microsoft is interested in having this cache inside the SDKs
One point to have in mind is that it's possible to add protocols to an existing class at runtime... so we might only be able to cache parts of the ConformsToProtocol logic (the part where we use reflection to check if a managed class implements a certain interface / has a certain attribute). |
Good point, there's a There's two parts that does reflection inside
So if we can cache only the reflection part we still win back 2/3 of the time. Or, since the protocols conformance can't be removed, use the cache and fall back to native/selectors when it returns |
@spouliot I remember the protocol checks pop up on mono/xammac, but not that often in net6 - but when using dotnet trace (on release builds) I don't see it. It's possible it's inlined? I guess it's the frame right under RemoveFromSuperview - this is a short trace taken after playing around in the IDE for a bit, |
Is the managed RemoveFromSuperview method called for direct bindings as well? |
@Therzok thanks for looking at it 😄
No, it could be removed (by the linker/trimmer) if the dynamic registrar is removed. I thought it was not for VSfM but I'm not 💯% sure about it... Also it's possible that
Yes, unless the linker/trimmer was able to remove it: like no |
I need to look into why I'm missing so many symbols, these stacks should all have had symbols in them. |
Note that the call to native code still _always_ happen (not cached) since the application could use `class_addProtocol` to add conformance to a protocol at runtime. So the cache is limited to the .net specific reflection code that is present (only) when the dynamic registrar is included inside applications. This is the default for macOS apps, but not iOS / tvOS or MacCatalyst apps. The linker/trimmer will remove the caching code when the dynamic registrar is removed. IOW this PR should not have any impact, performance or size, for most iOS apps (where the dynamic registrar is removed by default). Fix dotnet#14065 Running Dope on macOS, a 2 minutes benchmark, shows the following times (in seconds and percentage) spent calling this API: **Before** * `RemoveFromSuperview` 7.99s (6.4%) * `NSObject.ConformsToProtocol` 3.26s (2.6%) **After** * `RemoveFromSuperview` 4.67s (3.8%) * `NSObject.ConformsToProtocol` 0.32s (.26%) So a 10x improvements on `ConformsToProtocol` which helps a lot the code path calling `RemoveFromSuperview`.
Note that the call to native code still _always_ happen (not cached) since the application could use `class_addProtocol` to add conformance to a protocol at runtime. So the cache is limited to the .net specific reflection code that is present (only) when the dynamic registrar is included inside applications. This is the default for macOS apps, but not iOS / tvOS or MacCatalyst apps. The linker/trimmer will remove the caching code when the dynamic registrar is removed. IOW this PR should not have any impact, performance or size, for most iOS apps (where the dynamic registrar is removed by default). Fix #14065 Running Dope on macOS, a 2 minutes benchmark, shows the following times (in seconds and percentage) spent calling this API: ## Before <img width="1051" alt="Screen Shot 2022-06-15 at 9 21 22 PM" src="https://user-images.githubusercontent.com/260465/174201703-a91860a5-ec29-4e19-9de0-5158fd7aafa7.png"> * `RemoveFromSuperview` 7.99s (6.4%) * `NSObject.ConformsToProtocol` 3.26s (2.6%) ## After <img width="1228" alt="Screen Shot 2022-06-15 at 9 24 42 PM" src="https://user-images.githubusercontent.com/260465/174201708-92193e77-ea8e-41bc-9672-bddaaa18a4f6.png"> * `RemoveFromSuperview` 4.67s (3.8%) * `NSObject.ConformsToProtocol` 0.32s (.26%) So a 10x improvements on `ConformsToProtocol` which helps a lot the code path calling `RemoveFromSuperview`.
Steps to Reproduce
No specific step to reproduce, but it seems that UIView.AddSubview / UIView.RemoveFromSuperview with custom types inheriting from UIView seems to trigger this from time to time.
Expected Behavior
ConformsToProtocol
does not appear prominently in performance traces.Actual Behavior
Most of the time is spent here:
https://github.com/xamarin/xamarin-macios/blob/d78aa6712b15659b0fed259952364c060ffedcea/src/Foundation/NSObject2.cs#L407
A lot of the time is spent invoking this method at regular times, seemingly in batch, but there's no direct stack trace leading to invocation from managed code.
Environment
VS 2022 17.1 Preview 5
Build Logs
Example Project (If Possible)
I do not have an available project yet.
The text was updated successfully, but these errors were encountered: