Skip useless handler mappings calls while connecting #27259
Conversation
|
/azp run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
5573017 to
893e551
Compare
|
/azp run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
| internal static bool IsMappingProperties(this IElementHandler handler) => | ||
| (handler as IElementHandlerStateExhibitor)?.State.HasFlag(ElementHandlerState.MappingProperties) ?? false; |
There was a problem hiding this comment.
The method as-is is user-friendly. However, the result is perhaps unexpected when handler is not IElementHandlerStateExhibitor. In that case, I would expect to get null (and not false) as it would tell me "perhaps the handler is actually mapping properties but it does not provide the information". Though, I'm aware that that might lead to less friendly API.
Anyway, feel free to ignore if you feel like it does not add any value.
There was a problem hiding this comment.
This is only temporary for .NET9, and the ?. is there for type safety.
Hopefully these methods will be public and moved to IElementHandler in .NET10.
| /// <summary> | ||
| /// Maps the abstract <see cref="IBorderStroke.Shape"/> property to the platform-specific implementations. | ||
| /// </summary> | ||
| /// <param name="handler">The associated handler.</param> | ||
| /// <param name="border">The associated <see cref="IBorderView"/> instance.</param> | ||
| public static void MapStrokeShape(IBorderHandler handler, IBorderView border) | ||
| { | ||
| if (ShouldSkipStrokeMappings(handler)) return; |
There was a problem hiding this comment.
Nit but this makes putting breakpoints easier:
| if (ShouldSkipStrokeMappings(handler)) return; | |
| if (ShouldSkipStrokeMappings(handler)) | |
| { | |
| return; | |
| } |
|
Is this already available in nightlies? |
|
Definitely it would be good to try it. Great to see attention to the performances as MAUI struggles a lot there. |
Try this out and let us know: 9.0.40-ci.main.25073.66 |
|
@albyrock87 just tried the latest nightly, everything seems smooth! ( only tested so far on Android) |
do you have a repro I can test against to compare before and after this PR? Or possibly a video of what you mean by "everything seems smooth"? |
|
@PureWeen not really, since my app is quite complex (ble, views, etc) I haven't still recorded a speedscope, so it is really just more a "feeling" than any measurable data. In debug mode, and release mode, it seems the loads are a bit faster in pages Appearing, not dramatically faster, but seems faster. Once I have opportunity I will try to record something though |
|
@bcaceiro if you record the speedscopes for comparison, you should check timings on |
|
Update: this is not available on nightlies. So I really didn' test this PR in my previous "feeling". |
|
@MartyIX If I remember correctly in case of this page most of the delay is thanks to If you want I can try to create some example based more around changes in this PR but from what I've seen you already have one.
|
Description of Change
This is an alternative initial version of #27042.
Everything has already been explained there.
Credits to @MartyIX to have explored the effectiveness of skipping specific mappers.
Key differences from the other PR:
IElementandIElementHandlerto get the handler stateMappingFrametakes care of all the coordinates, even translations, while on other platforms the situation is different)Benchmark (from CI - on iOS UI tests)
Note: there is some variability in the benchmark caused by the runner itself, but looking at many recent PRs,
mainbranch never goes under 2600ms.mainbranch#27042 original PR
this
PRIssues Fixed
Fixes #17303