Skip to content

[android] call PlatformContentViewGroup.postInvalidate() less#24222

Merged
jonathanpeppers merged 2 commits into
dotnet:mainfrom
jonathanpeppers:setHasClip
Aug 14, 2024
Merged

[android] call PlatformContentViewGroup.postInvalidate() less#24222
jonathanpeppers merged 2 commits into
dotnet:mainfrom
jonathanpeppers:setHasClip

Conversation

@jonathanpeppers

Copy link
Copy Markdown
Member

Context: #23991

@chabiss's sample app is running a lot of GCs:

[riodictablemaui] Explicit concurrent copying GC freed 933(127KB) AllocSpace objects, 31(4092KB) LOS objects, 49% free, 4423KB/8846KB, paused 1.328ms,169us total 33.513ms
[riodictablemaui] Explicit concurrent copying GC freed 722(46KB) AllocSpace objects, 0(0B) LOS objects, 49% free, 4581KB/9162KB, paused 1.856ms,170us total 14.835ms
[riodictablemaui] Explicit concurrent copying GC freed 641(43KB) AllocSpace objects, 1(108KB) LOS objects, 49% free, 4525KB/9050KB, paused 1.420ms,176us total 14.112ms
[riodictablemaui] Explicit concurrent copying GC freed 619(41KB) AllocSpace objects, 0(0B) LOS objects, 49% free, 4579KB/9158KB, paused 1.263ms,173us total 13.825ms
... many many more lines

This doesn't seem like a normal amount of GCs for a simple app like this.

Attaching Android Studio's allocation profiler, I can see a significant amount of allocations at:

com.microsoft.maui.PlatformContentViewGroup.setHasClip()
    Allocations: 35,906
    Deallocations: 143
    Total Count: 35,763
    Shallow Size: 2,288,832

The call to postInvalidate() is running a lot, and this causes:

  • A layout cycle

  • android.os.Message to be allocated

  • etc. etc.

I think we can simply check if hasClip has changed, and only call postInvalidate() if it has.

Context: dotnet#23991

@chabiss's sample app is running a lot of GCs:

    [riodictablemaui] Explicit concurrent copying GC freed 933(127KB) AllocSpace objects, 31(4092KB) LOS objects, 49% free, 4423KB/8846KB, paused 1.328ms,169us total 33.513ms
    [riodictablemaui] Explicit concurrent copying GC freed 722(46KB) AllocSpace objects, 0(0B) LOS objects, 49% free, 4581KB/9162KB, paused 1.856ms,170us total 14.835ms
    [riodictablemaui] Explicit concurrent copying GC freed 641(43KB) AllocSpace objects, 1(108KB) LOS objects, 49% free, 4525KB/9050KB, paused 1.420ms,176us total 14.112ms
    [riodictablemaui] Explicit concurrent copying GC freed 619(41KB) AllocSpace objects, 0(0B) LOS objects, 49% free, 4579KB/9158KB, paused 1.263ms,173us total 13.825ms
    ... many many more lines

This doesn't seem like a normal amount of GCs for a simple app like this.

Attaching Android Studio's allocation profiler, I can see a significant
amount of allocations at:

    com.microsoft.maui.PlatformContentViewGroup.setHasClip()
        Allocations: 35,906
        Deallocations: 143
        Total Count: 35,763
        Shallow Size: 2,288,832

The call to `postInvalidate()` is running *a lot*, and this causes:

* A layout cycle

* `android.os.Message` to be allocated

* etc. etc.

I think we can simply check if `hasClip` has changed, and only call
`postInvalidate()` if it has.
@jonathanpeppers jonathanpeppers added perf/general The issue affects performance (runtime speed, memory usage, startup time, etc.) (sub: perf) platform/android labels Aug 13, 2024
@jonathanpeppers

Copy link
Copy Markdown
Member Author

I was testing this change with a Stopwatch:

public async Task RefreshView()
{
    if (this.IsRefreshing)
    {
        return;
    }

    Console.WriteLine($"RefreshView starting");
    var sw = new System.Diagnostics.Stopwatch();
    sw.Start();
    this.IsRefreshing = true;
    PeriodicTableDataModel dataModel = await this.GetDataModelAsync();
    if(dataModel != null)
    {
        var observable = new ObservableCollection<ElementViewModel>();
        foreach(var element in dataModel.Elements) 
        {
            observable.Add(new ElementViewModel(element));
        }
        this.Elements = observable;
        OnPropertyChanged(nameof(Elements));
    }
    this.IsRefreshing = false;

    sw.Stop();
    Console.WriteLine($"RefreshView took {sw.ElapsedMilliseconds} ms");
}

Before it was around 17 seconds debugging on a Pixel 7, with this change it's:

[DOTNET] RefreshView took 8569 ms

Using dotnet/maui/main, I see some log messages like this, so there might be other things we can improve:

[0:] Microsoft.Maui.Controls.Xaml.Diagnostics.BindingDiagnostics: Warning: Mismatch between the specified x:DataType and the current binding context

@jonathanpeppers jonathanpeppers marked this pull request as ready for review August 14, 2024 14:03
@jonathanpeppers jonathanpeppers requested a review from a team as a code owner August 14, 2024 14:03
@jonathanpeppers

Copy link
Copy Markdown
Member Author

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 3 pipeline(s).

@jonathanpeppers jonathanpeppers enabled auto-merge (squash) August 14, 2024 19:19
@jonathanpeppers jonathanpeppers merged commit 6c21d39 into dotnet:main Aug 14, 2024
@samhouts samhouts added fixed-in-net9.0-nightly This may be available in a nightly release! fixed-in-net8.0-nightly This may be available in a nightly release! labels Aug 27, 2024
@jonathanpeppers jonathanpeppers deleted the setHasClip branch September 28, 2024 16:39
albyrock87 added a commit to albyrock87/maui that referenced this pull request Sep 29, 2024
rmarinho pushed a commit that referenced this pull request Oct 8, 2024
…ad of `postInvalidate` (#25121)

* Revert #24222 and simply use `invalidate` instead of `postInvalidate`

* - update aar file

---------

Co-authored-by: Alberto Aldegheri <albyrock87@gmail.com>
Co-authored-by: Shane Neuville <shneuvil@microsoft.com>
github-actions Bot pushed a commit to albyrock87/maui that referenced this pull request Oct 8, 2024
github-actions Bot pushed a commit to albyrock87/maui that referenced this pull request Oct 10, 2024
@github-actions github-actions Bot locked and limited conversation to collaborators Oct 29, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

fixed-in-net8.0-nightly This may be available in a nightly release! fixed-in-net9.0-nightly This may be available in a nightly release! perf/general The issue affects performance (runtime speed, memory usage, startup time, etc.) (sub: perf) platform/android

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants