[release/8.0.1xx-rc1] Merge net8.0 to 8.0.1xx-rc1#17007
Merged
rmarinho merged 44 commits intorelease/8.0.1xx-rc1from Aug 27, 2023
Merged
[release/8.0.1xx-rc1] Merge net8.0 to 8.0.1xx-rc1#17007rmarinho merged 44 commits intorelease/8.0.1xx-rc1from
rmarinho merged 44 commits intorelease/8.0.1xx-rc1from
Conversation
Context: #16436 Context: https://github.com/symbiogenesis/Maui.DataGrid/tree/net8-memory-leak A customer sample has a `CollectionView` with 150,000 data-bound rows. Debugging what happens at runtime, I found we effectively were doing: _itemTemplateContexts = new List<ItemTemplateContext>(capacity: 150_000); for (int n = 0; n < 150_000; n++) { _itemTemplateContexts.Add(null); } Then the items were created as you scroll each row into view: if (_itemTemplateContexts[index] == null) { _itemTemplateContexts[index] = context = new ItemTemplateContext(_itemTemplate, _itemsSource[index], _container, _itemHeight, _itemWidth, _itemSpacing, _mauiContext); } return _itemTemplateContexts[index]; This code accesses the indexer multiple times, into a `List<T>` with 150,000 `null` items. To improve this: * use a `Dictionary<int, T>` instead, just let it size dynamically. * use `TryGetValue(..., out var context)`, so each call accesses the indexer one less time than before. * use either the bound collection's size or 64 (whichever is smaller) as a rough estimate of how many might fit on screen at a time. Taking a memory snapshot of the app after startup: Before: Heap Size: 82,899.54 KB After: Heap Size: 81,768.76 KB Which is saving about 1MB of memory on launch. In this case, it feels better to just let the `Dictionary` size itself with an estimate of what `capacity` will be.
Context: https://github.com/jonathanpeppers/memory-analyzers Context: https://www.nuget.org/packages/MemoryAnalyzers/0.1.0-beta.3 This adds a new Roslyn analyzer that warns about the following cases. ## MA0001 Don't define `public` events in `NSObject` subclasses: ```csharp public class MyView : UIView { // NOPE! public event EventHandler MyEvent; } ``` ## MA0002 Don't declare members in `NSObject` subclasses unless they are: * `WeakReference` or `WeakReference<T>` * Value types ```csharp class MyView : UIView { // NOPE! public UIView? Parent { get; set; } public void Add(MyView subview) { subview.Parent = this; AddSubview(subview); } } ``` ## MA0003 Don't subscribe to events inside `NSObject` subclasses unless: * It's your event via `this.MyEvent` or from a base type. * The method is `static`. ```csharp class MyView : UIView { public MyView() { var picker = new UIDatePicker(); AddSubview(picker); picker.ValueChanged += OnValueChanged; } void OnValueChanged(object sender, EventArgs e) { } // Use this instead and it doesn't leak! //static void OnValueChanged(object sender, EventArgs e) { } } ``` This is also on NuGet, but I just commited the package until we can get it added to the `dotnet-public` feed. Places with PRs in flight are marked with: [UnconditionalSuppressMessage("Memory", "MA0002", Justification = "FIXME: #16530")] A few are marked as "not an issue" with an explanation. Others mention a test with a proof they are OK. A few places I could actually *remove* `UnconditionalSuppressMessage` where I could improve the analyzer to ignore that case.
…30816.1 (#16890) Microsoft.DotNet.XHarness.CLI , Microsoft.DotNet.XHarness.TestRunners.Common , Microsoft.DotNet.XHarness.TestRunners.Xunit From Version 8.0.0-prerelease.23407.2 -> To Version 8.0.0-prerelease.23416.1 Co-authored-by: dotnet-maestro[bot] <dotnet-maestro[bot]@users.noreply.github.com>
* Set Mapper/Handler ctor to proected on migration handlers * - fix Tizen APIs
* [WinUI] Update shadow mask as part of shadow properties The shadow mask needs to be updaded with the shadow properties, because if the size or shape of the object changed the mask needs to be updated accordingly. This fixes a regression, but brings back another problem: shadows won't work for borders with no content (#13944). The workaround for that scenario is to add content or a background color to the border. Fixes #16094 * Use FireAndForget instead of async void * Fix spacing issues
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
* Initial work on adding connected component analysis to tests * Fix tests * Simplify test code * Code doc and cleanup * Move boxview tests to layout tests This is really a Grid test * Remove extranous linefeed * Disable tests for now * Remove license headers * Remove unused code --------- Co-authored-by: Morten Nielsen <mort5161@esri.com>
* Improve the control over resource generation * Improve the control over resource generation * From comments * feedback
* First set of comments * More comments * Finish! * PR Feedback * More PR feedback * Apply suggestions from code review Co-authored-by: Samantha Houts <samhouts@users.noreply.github.com> --------- Co-authored-by: Samantha Houts <samhouts@users.noreply.github.com>
* Add property for closing softkeyboard when tapping on page * - add android behavior * - add public API for tap outside * - cleanup code * - remove changes * - cleanup * - fix iOS * - add keyboard testing * - fix setup code * - cleanup code and tests * - scope HideSoftInput to a service and simplify * - fix up iOS * - ios fixes * - fix test * - move iOS Test * Docs nit * - fix ios * - cleanup loaded code * - fix * - set after frame is set * - additional timing fixes * - remove tests covered by the UI tests * - fix ios SearchHandler * Update HideSoftInputOnTappedPageTests.cs * - pop any dangling modals * - simplify down the code * - wire up * - rollback VE changes * - cleanup * - remove empty space * - remove excessive loading code for now * - remove changes on page * Update src/Core/src/Core/IPlatformEventsListener.cs * Update src/Core/src/Core/IPlatformEventsListener.cs * - fix keyboard APIs --------- Co-authored-by: Juan Diego Herrera <juherrera@microsoft.com> Co-authored-by: Samantha Houts <samhouts@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
* [appium] Bump Versions * [appium] Some fixes to the script * [appium] Install also appium-doctor * [provisioning] Allow skip provision all android api sdks * Fix * [cake] Bump nuget tool * [uitests] Comment out CarouselView tests * Remove grep doesn't exist on windows
* [windows] Added ArrangeOverride override and changed MeasureOverride in ItemContentControl to not call base when the ItemHeight and ItemWidth are defined When the height and width of the items are already defined, it's likely that the base.MeasureOverride (WinUI) call reduces the original size, causing issues like the items in a CarouselView not fitting the container, which causes more items to be displayed at once. This fix makes sure that we always return the max height and width between the measured override and the existing items height and width. Also, adding ArrangeOverride from WinUI base class to control the layout correctly and completely on our side. Fixes Bug: #12567 * Added Appium test for CarouselView to validate fix for bug #12567 * Fix RS0016 + added inheritdoc * Fixed return statement on ArrangeOverride according to PR feedback * Fixed CarouselViewHandler resizing logic according to PR feedback * Attempt to fix test errors --------- Co-authored-by: Juan Diego Herrera <juherrera@microsoft.com>
When building on macOS I've been hitting a workload install issue:
Workload installation failed. Rolling back installed packs...
Rolling back pack Microsoft.Maui.Graphics.Win2D.WinUI.Desktop installation...
Workload installation failed: microsoft.maui.graphics.win2d.winui.desktop::7.0.100-dev is not found in NuGet feeds ...
The project that produces this pack doesn't seem to be built on macOS,
and it's not available in the local artifacts feed after a build.
We can take advantage of [alias packs][0] to fix this issue. This will
also simplify workload installs for customers not on Windows, as the
pack will now be ignored entirely on other platforms.
[0]: https://github.com/dotnet/designs/blob/37718c66f403f7a5d25517cdac3ec2d2553100a6/accepted/2020/workloads/workload-manifest.md#alias-packs
* [core] use factory methods for registering services
In .NET 8 Preview 7, we noticed a regression in startup around:
.NET 8 Preview 6:
45.35ms microsoft.maui!Microsoft.Maui.Hosting.MauiAppBuilder.Build()
.NET 8 Preview 7:
62.17ms microsoft.maui!Microsoft.Maui.Hosting.MauiAppBuilder.Build()
This may have been related to:
dotnet/runtime#87183
Which should be improved by:
dotnet/runtime#89964
In reviewing the traces, we noticed several places where MAUI was
registering services like:
services.AddTransient<IFoo, Foo>();
Where we could instead do:
services.AddTransient<IFoo>(_ => new Foo());
Where this registration avoids any System.Reflection work at startup.
Microsoft.Extensions.DI can just call the factory method instead.
I expanded upon `BannedSymbols.txt` like we did in e7812b0, to ban
cases of `AddTransient` and `AddScoped` that might be used accidentally.
This resulted in banning the slow versions of these APIs like:
Microsoft.Maui.Hosting.ImageSourceServiceCollectionExtensions.AddService
Microsoft.Maui.Hosting.MauiHandlersCollectionExtensions.AddHandler
Microsoft.Maui.Hosting.MauiHandlersCollectionExtensions.TryAddHandler
I also introduced public, fast versions of methods in
`MauiHandlersCollectionExtensions`.
With this change in place, I could see the difference in:
Before:
11.24ms microsoft.extensions.dependencyinjection!Microsoft.Extensions.DependencyInjection.ServiceLookup.DynamicServiceProviderEngine..ctor
After:
6.29ms microsoft.extensions.dependencyinjection!Microsoft.Extensions.DependencyInjection.ServiceLookup.DynamicServiceProviderEngine..ctor
Which resulted in an overall faster startup of a `dotnet new maui` app
on a Pixel 5:
Average(ms): 691
Std Err(ms): 3.00370142028502
Std Dev(ms): 9.49853789918334
Average(ms): 687.8
Std Err(ms): 4.04365071576553
Std Dev(ms): 12.7871463239892
This is an average of 10 runs. Note that this was built with main, and
so we have an out-of-date AOT profile compared to the `net8.0` branch.
* Remove CellRenderer registration
Co-authored-by: Rui Marinho <me@ruimarinho.net>
* Handlers mustn't implement IImageSourcePartSetter * Safer * Updates per feedback
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
…View (#16958) When having a `CollectionView` with a Linear horizontal layout in a `Grid` with a row definition set to Auto, adding an item would make rezising the inner `ItemsWrapGrid` enter in an inifinite loop making the height grow indefinetly. The same would happen for for vertical/column/width. The problem was that when using a linear layout the wrap grid should never have more than one row (it shouldn't wrap the items after exceding its size), regardless of the space available, but the wrap grid was not aware of this. The cycle happened when the wrap grid total height (or width, depending on the orientation) increased, so then we update the item height, but as span is 1 because of linear layout, we ended up setting the total height as item height, forcing the wrap grid to grow again. The fix is simply configure the `ItemsWrapGrid` to restrict the number of rows/columns it can have based on what's set for Span. Fixes #16320
* [android] update AOT profile for .NET 8.0.0-rc.1.23419.3
I recorded this profile using newer builds of .NET 8 RC 1.
An average of 10 runs of a dotnet new maui app on a Pixel 5, the new
profile helps quite a bit:
Before:
Average(ms): 693.2
Std Err(ms): 5.25314720482451
Std Dev(ms): 16.6119100513925
After:
Average(ms): 607.8
Std Err(ms): 2.94316534061922
Std Dev(ms): 9.307106006822
Things appear to be even slower than last time in 3f22153. 😢
* Use https://httpbin.org/status/200
https://httpstat.us/200 appears to be down right now.
The new profile is slightly better:
Average(ms): 594.6
Std Err(ms): 3.06304133537604
Std Dev(ms): 9.68618718703197
But still appears to be a smidge worse than 3f22153.
* Add interface to expose out UIView methods * - add to API txt files and clean up * - fix test timing to only fire after page has finished layout * - cleanup code * - infest all the places * - fix tests to use ToPlatform * - cleanup code * - fix accessibility test * - fix accessibility checks to use * - fix justifications
### Description of Change Merge latest changes from main to net8
MenuShellItem wraps a MenuItem. Shell related properties should be synchronized between the two objects, always the same values. Add code so that FlyoutItemIsVisible is also synchronized. Most properties are synchroned via bindings, but FlyoutItemIsVisible is an attached property so we need to have code that runs on property changes instead. Fixes issue #16929 Note that normally the MenuItem is specified in XAML while MenuShellItem is accessed in code behind, thus the need to synchronize when FlyoutItemIsVisible is initialized in XAML then updated later via code behind, like in the bug.
* Use a fixed WinUI window size for Appium test app This makes the screenshots more deterministic, more likely to match between dev machines and CI. * Also fix window size for catalyst * Update size to 1024x768 and fix catalyst * Crop title bar off for WinUI screenshots * Force light theme for WinUI, so screenshots are deterministc * Update baseline WinUI images, for new deterministic settings * Fix build warning on non-desktop platforms * Add mission Issue16094 iOS snapshot file * Add mission Issue16094 Android snapshot file
* Default paths for Appium projects, to simplify run steps Default the paths for UITest projects so that the user doesn't need to specify them manually. Once this is merged I'll update the instructions in https://github.com/dotnet/maui/wiki/UITests to remove the --project and --appproject arguments from the run steps. That makes them much simpler and avoids the issue of them being hardcoded to Rui's home directory. * Undo one of the windows changes, as breaks devices tests * Only apply defaults when target is uitest Also, improve error messages for windows/catalyst/ios similar to android, giving a user friendly error if the exe/app to be tested isn't found (likely because it's not built), instead of an NRE.
* Add Windows unpackaged DeviceTests in CI * Update device-tests.yml
rmarinho
requested review from
PureWeen and
mattleibow
and removed request for
a team
Member
Author
Eilon
previously approved these changes
Aug 25, 2023
Eilon
added
the
area-infrastructure
### Description of Change Bring latest changes from main to net8 branch
Redth
approved these changes
Aug 27, 2023
PureWeen
approved these changes
Aug 27, 2023
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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
14 participants
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.
Description of Change
Bring latest changes from net8 to the release 8.0.1xx-rc1