Skip to content
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.

Sign up for GitHub

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

Jump to bottom

perf: Apply various workarounds to mitigate PG-AOT dotnet/runtime issue #56309 #7005

Merged
merged 1 commit into from
Sep 9, 2021
Merged

perf: Apply various workarounds to mitigate PG-AOT dotnet/runtime issue #56309 #7005

merged 1 commit into from
Sep 9, 2021

Conversation

jeromelaban
Copy link
Member

GitHub Issue (If applicable): dotnet/runtime#56309

PR Type

What kind of change does this PR introduce?

  • Refactoring (no functional changes, no api changes)

What is the new behavior?

These changes are required to reduce the interpreter performance hit. The AOT compiler detects try/catch/finally containing methods and forces the interpreter, even if the try/catch is not in the execution path.

Inner methods are created to reduce the size of the interpreter-executed code, and return to AOT as soon as possible.

PR Checklist

Please check if your PR fulfills the following requirements:

Other information

Internal Issue (If applicable):

@jeromelaban jeromelaban requested a review from a team September 9, 2021 14:54
@gitpod-io
Copy link

gitpod-io bot commented Sep 9, 2021

@jeromelaban
perf: Apply various workarounds for dotnet/runtime issue #56309
5cbbec2 
These changes are required to reduce the interpreter performance hit.  The AOT compiler detects try/catch/finally containing methods and forces the interpreter, even if the try/catch is not in the execution path.

Inner methods are created to reduce the size of the interpreter-executed code, and return to AOT as soon as possible.
@jeromelaban jeromelaban force-pushed the dev/jela/try-catch-mitigate branch from 68bd3b7 to 5cbbec2 Compare September 9, 2021 15:50
carldebilly
carldebilly approved these changes Sep 9, 2021
View reviewed changes
Copy link
Member

@carldebilly carldebilly left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TL;JA

@jeromelaban jeromelaban added the ready-to-merge Automatically merge the PR once all '.mergify.yml' policies are met label Sep 9, 2021
@ajpinedam
Copy link
Contributor

TL;JA

@carldebilly does this mean: Too long J'ai approved?

@carldebilly
Copy link
Member

TL;JA

@carldebilly does this mean: Too long J'ai approved?

Too Long; Just Approved ;-)

@nventive-devops
Copy link
Contributor

The build 33134 found UI Test snapshots differences: android-28-net6: 0, android-28-xamarin: 19, android-28-xamarin-Snap: 20, ios: 11, ios-Snap: 45, skia-gtk-screenshots: 31, wasm: 14, wasm-automated-net5.0-Benchmarks: 0, wasm-automated-net5.0-Default: 4, wasm-automated-net5.0-RuntimeTests: 0

Details

  • android-28-net6: 0 changed over 1

  • android-28-xamarin: 19 changed over 730

    🚨🚨 Comparison Details (first 20) 🚨🚨
    • ComboBoxTests_Fullscreen_Popup_Generic_Closed
    • ComboBoxTests_Fullscreen_Popup_Generic_Opened
    • ComboBox_Dropdown_Background_ComboBox_open
    • DatePicker_PickerFlyout_PlacementsbottomRight_True_False_True_UITests_Windows_UI_Xaml_Controls_DatePicker_DatePicker_Placement
    • FocusManager_FocusDirection_Previous_Validation_Uno_UI_Samples_Content_UITests_FocusTests_FocusManager_FocusDirection
    • PasswordBox_AutoGrow_Horizontally_Test_Uno_UI_Samples_Content_UITests_TextBoxControl_PasswordBox_Simple
    • PopupWithOverlay_After
    • ProgressRing_IsEnabled_Running_UITests_Windows_UI_Xaml_Controls_ProgressRing_WindowsProgressRing_GH1220
    • ProgressRing_Visibility_Collapsed_UITests_Windows_UI_Xaml_Controls_ProgressRing_WindowsProgressRing_GH1220
    • ScrollViewer_Removed_And_Added_Initial-Scrolled
    • SequentialAnimations_SamplesApp_Windows_UI_Xaml_Media_Animation_SequentialAnimationsPage
    • Validate_Offscreen_Shapes_UITests_Windows_UI_Xaml_Shapes_Offscreen_Shapes
    • WebView_NavigateToAnchor_navigate_to_anchor
    • WebView_NavigateToLongString_AfterSuccess
    • When_Click_Outside_Dialog_Expect_No_Dismiss_AfterClicking
    • When_Click_Outside_Dialog_Expect_No_Dismiss_BeforeClicking
    • When_Parent_PointerMoved_After_drag_on_non-scrolling_ScrollViewer
    • When_Parent_PointerMoved_After_drag_on_ScrollViewer_-_touch
    • When_TransformToVisual_ScrollViewer_UITests_Shared_Windows_UI_Xaml_UIElementTests_TransformToVisual_ScrollViewer

  • android-28-xamarin-Snap: 20 changed over 913

    🚨🚨 Comparison Details (first 20) 🚨🚨
    • ColorPicker_ColorPickerSample_UITests_Microsoft_UI_Xaml_Controls_ColorPickerTests_ColorPickerSample
    • Default_UITests_Shared_Windows_UI_Xaml_Automation_AutomationProperties_Name_UITests_Shared_Windows_UI_Xaml_Automation_AutomationProperties_Name
    • ComboBox_UITests_Shared_Windows_UI_Xaml_Controls_ComboBox_ComboBox_DropDownPlacement_UITests_Shared_Windows_UI_Xaml_Controls_ComboBox_ComboBox_DropDownPlacement
    • GridTestsControl_CenteredGridinGridwiththreefixedsizechildren_Uno_UI_Samples_Content_UITests_GridTestsControl_CenteredGridinGridwiththreefixedsizechildren
    • Image_UITests_Windows_UI_Xaml_Controls_ImageTests_Image_Formats_UITests_Windows_UI_Xaml_Controls_ImageTests_Image_Formats
    • ListView_ListViewGroupedLargeLegacy_SamplesApp_Windows_UI_Xaml_Controls_ListView_ListViewGroupedLargeLegacy
    • NavigationView_MUXControlsTestApp_NavigationViewPage_MUXControlsTestApp_NavigationViewPage
    • NavigationView_MUXControlsTestApp_HierarchicalNavigationViewMarkup_MUXControlsTestApp_HierarchicalNavigationViewMarkup
    • Progress_UITests_Microsoft_UI_Xaml_Controls_ProgressBar_WinUIProgressBarSimple_UITests_Microsoft_UI_Xaml_Controls_ProgressBar_WinUIProgressBarSimple
    • Progress_UITests_Microsoft_UI_Xaml_Controls_ProgressRing_WinUIProgressRingPage_UITests_Microsoft_UI_Xaml_Controls_ProgressRing_WinUIProgressRingPage
    • Progress_UITests_Microsoft_UI_Xaml_Controls_ProgressRing_WinUIProgressRing_CustomSources_UITests_Microsoft_UI_Xaml_Controls_ProgressRing_WinUIProgressRing_CustomSources
    • Progress_UITests_Windows_UI_Xaml_Controls_ProgressRing_WindowsProgressRing_GH1220_UITests_Windows_UI_Xaml_Controls_ProgressRing_WindowsProgressRing_GH1220
    • Shapes_UITests_Windows_UI_Xaml_Shapes_Offscreen_Shapes_UITests_Windows_UI_Xaml_Shapes_Offscreen_Shapes
    • TabView_UITests_Microsoft_UI_Xaml_Controls_TabViewTests_TabViewPage_UITests_Microsoft_UI_Xaml_Controls_TabViewTests_TabViewPage
    • TextBox_Uno_UI_Samples_Content_UITests_TextBoxControl_PasswordBox_Reveal_Scroll_Uno_UI_Samples_Content_UITests_TextBoxControl_PasswordBox_Reveal_Scroll
    • TextBox_Uno_UI_Samples_Content_UITests_TextBoxControl_TextBox_In_ContentDialog_Uno_UI_Samples_Content_UITests_TextBoxControl_TextBox_In_ContentDialog
    • TimePicker_UITests_Windows_UI_Xaml_Controls_TimePicker_TimePicker_Header_UITests_Windows_UI_Xaml_Controls_TimePicker_TimePicker_Header
    • Time_Picker_TimePicker_TimePickerFlyoutStyle_UITests_Windows_UI_Xaml_Controls_TimePicker_TimePicker_TimePickerFlyoutStyle
    • WebView_WebView_WithHeaders_Uno_UI_Samples_Content_UITests_WebView_WebView_WithHeaders
    • UIElement_TransformToVisual_Simple_UITests_Shared_Windows_UI_Xaml_UIElementTests_TransformToVisual_Simple

  • ios: 11 changed over 275

    🚨🚨 Comparison Details (first 20) 🚨🚨
    • ImageStretch_None_Uno_UI_Samples_UITests_ImageTestsControl_Image_Stretch_None
    • Keyboard_Textbox_InsideScrollViewer_Validation_2_-_Remove_Focus_on_placeholderTextTextBox
    • Keyboard_Textbox_InsideScrollViewer_Validation_0_-_Remove_Focus_on_normalTextBox
    • Keyboard_Textbox_NoScrollViewer_Validation_4_-_Remove_Focus_on_multilineTextBox
    • Keyboard_Textbox_NoScrollViewer_Validation_5_-_Remove_Focus_on_numberTextBox
    • ProgressRing_Visibility_Collapsed_UITests_Windows_UI_Xaml_Controls_ProgressRing_WindowsProgressRing_GH1220
    • SequentialAnimations_SamplesApp_Windows_UI_Xaml_Media_Animation_SequentialAnimationsPage
    • TextBox_RoundedCorners_UITests_Shared_Windows_UI_Xaml_Controls_TextBoxTests_TextBox_RoundedCorners
    • When_CustomContentAndLongTitleAndDoubleNavigation_TitleShouldNotOverlapBarButtons_OnNavigateBack_NativeFrame_UITests_Windows_UI_Xaml_Controls_CommandBar_LongTitle_CommandBar_Frame
    • UIElement_ContextFlyout_opened
    • UIElement_ContextFlyout_UITests_Shared_Windows_UI_Xaml_Controls_MenuFlyoutTests_UIElement_ContextFlyout

  • ios-Snap: 45 changed over 904

    🚨🚨 Comparison Details (first 20) 🚨🚨
    • Animations_DoubleAnimationUsingKeyFrames_TranslateX_GenericApp_Views_Content_UITests_Animations_DoubleAnimationUsingKeyFrames_TranslateX
    • Animations_DoubleAnimation_Cumulative_Uno_UI_Samples_Content_UITests_Animations_DoubleAnimation_Cumulative
    • Animations_DoubleAnimation_RenderTransformOrigin_GenericApp_Views_Content_UITests_Animations_DoubleAnimation_RenderTransformOrigin
    • Automation_AutomationProperties_AutomationId_UITests_Shared_Windows_UI_Xaml_Automation_AutomationProperties_AutomationId
    • AutoSuggestBox_UITests_Windows_UI_Xaml_Controls_AutoSuggestBoxTests_AutoSuggestBox_Icons_UITests_Windows_UI_Xaml_Controls_AutoSuggestBoxTests_AutoSuggestBox_Icons
    • Border_AutoBorderStretchwithbottommargin_Uno_UI_Samples_UITests_BorderTestsControl_AutoBorderStretchwithbottommargin
    • AutoSuggestBox_UITests_Windows_UI_Xaml_Controls_AutoSuggestBoxTests_AutoSuggestBox_BitmapIcon_UITests_Windows_UI_Xaml_Controls_AutoSuggestBoxTests_AutoSuggestBox_BitmapIcon
    • Border_Border_CornerRadius_Clipping_UITests_Windows_UI_Xaml_Controls_BorderTests_Border_CornerRadius_Clipping
    • Border_Border_Simple_No_Background_With_TextBox_Uno_UI_Samples_UITests_BorderTestsControl_Border_Simple_No_Background_With_TextBox
    • ComboBox_ComboBox_NativePopup_SamplesApp_Wasm_Windows_UI_Xaml_Controls_ComboBox_ComboBox_NativePopup
    • Default_UITests_Windows_UI_Xaml_Controls_ListView_ListViewTemplateRootIsContainer_UITests_Windows_UI_Xaml_Controls_ListView_ListViewTemplateRootIsContainer
    • ImageBrushTestControl_PanelImageBrush_Uno_UI_Samples_UITests_ImageBrushTestControl_PanelImageBrush
    • Image_UITests_Shared_Windows_UI_Xaml_Controls_ImageTests_Image_Stretch_Algmnt_Inf_Vertical_UITests_Shared_Windows_UI_Xaml_Controls_ImageTests_Image_Stretch_Algmnt_Inf_Vertical
    • Image_UITests_Shared_Windows_UI_Xaml_Controls_ImageTests_Image_Stretch_Alignment_SizeOnControl_UITests_Shared_Windows_UI_Xaml_Controls_ImageTests_Image_Stretch_Alignment_SizeOnControl
    • ListView_ListViewSelectedItems_SamplesApp_Windows_UI_Xaml_Controls_ListView_ListViewSelectedItems
    • Image_Uno_UI_Samples_UITests_Image_Image_Stretch_Alignment_Bigger_Uno_UI_Samples_UITests_Image_Image_Stretch_Alignment_Bigger
    • Image_Uno_UI_Samples_UITests_Image_Image_Stretch_Alignment_Equal_Uno_UI_Samples_UITests_Image_Image_Stretch_Alignment_Equal
    • Image_Uno_UI_Samples_UITests_Image_Image_Stretch_Alignment_Smaller_Uno_UI_Samples_UITests_Image_Image_Stretch_Alignment_Smaller
    • Image_Uno_UI_Samples_UITests_Image_Image_Stretch_Alignment_Wider_Uno_UI_Samples_UITests_Image_Image_Stretch_Alignment_Wider
    • Image_Uno_UI_Samples_UITests_Image_Image_Stretch_Uno_UI_Samples_UITests_Image_Image_Stretch

  • skia-gtk-screenshots: 31 changed over 952

    🚨🚨 Comparison Details (first 20) 🚨🚨
    • Gesture recognizer-Pointer Events test bench
    • Image-Image_Stretch_Algmnt_Inf_Vertical
    • Image-Image_Stretch_Alignment_Bigger
    • Image-Image_Stretch_Alignment_Taller
    • Image-UniformToFill100x100
    • ListView-ListView_IsSelected
    • NavigationView-NavigationViewItemTemplatePage
    • NavigationView-NavigationViewMenuItemStretchPage
    • NavigationView-NavigationViewSample
    • NavigationView-PaneLayoutTestPage
    • Progress-WinUIDeterminateProgressRing
    • ScrollBar-ScrollBar_Simple
    • ScrollViewer-ScrollViewer_SnapPoints
    • Shapes-Offscreen_Shapes
    • TabView-TabViewBasicPage
    • TextBlockControl-TextBlock_LayoutAlignment
    • ToggleSwitch-ToggleSwitch_Header
    • ToggleSwitch-ToggleSwitch_IsOn
    • Transform-Border_With_Off_Centre_ScaleTransform
    • Transform-TextBox_With_RotateTransform

  • wasm: 14 changed over 913

    🚨🚨 Comparison Details (first 20) 🚨🚨
    • SamplesApp.Wasm.Windows_UI_Xaml_Controls.ListView.ListView_IsSelected
    • SamplesApp.Wasm.Windows_UI_Xaml_Media.Transform.TransformToVisual_Translate2d
    • UITests.Microsoft_UI_Xaml_Controls.ColorPickerTests.ColorPickerSample
    • UITests.Microsoft_UI_Xaml_Controls.ProgressRing.WinUIProgressRingPage
    • UITests.Shared.Windows_UI_Xaml_Controls.ImageTests.Image_Stretch_Algmnt_Inf_Vertical
    • UITests.Shared.Windows_UI_Xaml_Controls.ImageTests.Image_Stretch_Alignment_SizeOnControl
    • UITests.Windows_UI_Xaml.DragAndDrop.DragDrop_TestPage
    • UITests.Windows_UI_Xaml_Shapes.Offscreen_Shapes
    • Uno.UI.Samples.Content.UITests.CommandBar.CommandBar_Native
    • Uno.UI.Samples.Samples.Shared.Content.UITests.ImageBrushInList
    • Uno.UI.Samples.UITests.Image.Image_Margin
    • Uno.UI.Samples.UITests.Image.Image_Stretch_Alignment_Bigger
    • Uno.UI.Samples.UITests.Image.Image_Stretch_Alignment_Wider
    • Uno.UI.Samples.UITests.ImageBrushTestControl.Ellipse

  • wasm-automated-net5.0-Benchmarks: 0 changed over 1

  • wasm-automated-net5.0-Default: 4 changed over 802

    🚨🚨 Comparison Details (first 20) 🚨🚨
    • ImageStretch_None_Uno_UI_Samples_UITests_ImageTestsControl_Image_Stretch_None
    • Screenshots_Image_Stretch_Alignment_Equal_Mode-0
    • Validate_Offscreen_Shapes_UITests_Windows_UI_Xaml_Shapes_Offscreen_Shapes
    • When_TransformToVisual_ScrollViewer_UITests_Shared_Windows_UI_Xaml_UIElementTests_TransformToVisual_ScrollViewer

  • wasm-automated-net5.0-RuntimeTests: 0 changed over 1

@jeromelaban jeromelaban merged commit 7061278 into master Sep 9, 2021
@jeromelaban jeromelaban deleted the dev/jela/try-catch-mitigate branch September 9, 2021 22:00
jeromelaban added a commit that referenced this pull request Sep 10, 2021
@jeromelaban
perf: Additional changes to mitigate PG-AOT dotnet/runtime issue #56309
a852261 
See #7005 for additional details
jeromelaban added a commit that referenced this pull request Sep 10, 2021
@jeromelaban
perf: Additional changes to mitigate PG-AOT dotnet/runtime issue #56309
ba58785 
See #7005 for additional details
jeromelaban added a commit that referenced this pull request Sep 11, 2021
@jeromelaban
perf: Additional changes to mitigate PG-AOT dotnet/runtime issue #56309
218fd00 
See #7005 for additional details
This was referenced Sep 11, 2021
Merged
Merged
Merged
MartinZikmund pushed a commit to MartinZikmund/Uno that referenced this pull request Sep 24, 2021
@jeromelaban @MartinZikmund
perf: Additional changes to mitigate PG-AOT dotnet/runtime issue #56309
a688e0b 
See unoplatform#7005 for additional details
MartinZikmund pushed a commit to MartinZikmund/Uno that referenced this pull request Sep 24, 2021
@jeromelaban @MartinZikmund
perf: Additional changes to mitigate PG-AOT dotnet/runtime issue #56309
b184c61 
See unoplatform#7005 for additional details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dr1rrb dr1rrb dr1rrb approved these changes

@carldebilly carldebilly carldebilly approved these changes

@ajpinedam ajpinedam ajpinedam approved these changes

@davidjohnoliver davidjohnoliver Awaiting requested review from davidjohnoliver

@ebariche ebariche Awaiting requested review from ebariche

Assignees
No one assigned
Labels
ready-to-merge Automatically merge the PR once all '.mergify.yml' policies are met
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@jeromelaban @ajpinedam @carldebilly @nventive-devops @dr1rrb