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.

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

why it always call gc collection make web run slower. #23701

Closed
srxqds opened this issue Jul 6, 2020 · 11 comments
Closed

why it always call gc collection make web run slower. #23701

srxqds opened this issue Jul 6, 2020 · 11 comments
Labels
area-blazor Includes: Blazor, Razor Components feature-blazor-wasm This issue is related to and / or impacts Blazor WebAssembly ✔️ Resolution: Answered Resolved because the question asked by the original author has been answered. Status: Resolved

Comments

@srxqds
Copy link

srxqds commented Jul 6, 2020

Describe the bug

why always call gc collection?
image

To Reproduce

Exceptions (if any)

Further technical details

  • ASP.NET Core version: 3.2
  • Include the output of dotnet --info: 3.1.4
  • The IDE (VS / VS Code/ VS4Mac) you're running on, and it's version
@srxqds srxqds changed the title why it always call gc collection make run slower. why it always call gc collection make web run slower. Jul 6, 2020
@SteveSandersonMS
Copy link
Member

Garbage collection happens when lots of objects have been allocated and memory has to be cleared up. Collection is unlikely to be a problem in itself, however it might indicate that you do have a problem with your code allocating too many objects.

If you're able to supply some minimal repro code showing what's giving you performance issues, we might be able to make suggestions about how to improve it.

@SteveSandersonMS SteveSandersonMS added area-blazor Includes: Blazor, Razor Components feature-blazor-wasm This issue is related to and / or impacts Blazor WebAssembly Needs: Author Feedback The author of this issue needs to respond in order for us to continue investigating this issue. labels Jul 6, 2020
@srxqds
Copy link
Author

srxqds commented Jul 8, 2020

I will try to upload test case later, thank you.

@ghost ghost added Needs: Attention 👋 This issue needs the attention of a contributor, typically because the OP has provided an update. and removed Needs: Author Feedback The author of this issue needs to respond in order for us to continue investigating this issue. labels Jul 8, 2020
@srxqds
Copy link
Author

srxqds commented Jul 8, 2020

here are my app code:

@page "/Console"

@using Hunter.App.Shared.Models
@using Hunter.App.Shared
@using Hunter.App.Client.Services
@using Microsoft.AspNetCore.SignalR.Client;
@using WebAssembly.Browser.DOM;
@inject NavigationManager NavigationManager
@inject ConsoleHubClient consoleHub

<div class="hunterContainer">
    <div class="hunterDevList">
        <p>请选择要跟踪的客户端</p>
        <hr />
        @foreach (var clientData in _clientDataList)
        {
            <div class="hunterDevice @((clientData == this._curClientData? " selected" : ""))" @onclick="@(e => ListenToDevice(clientData.uid))">
                <p class="hunterDeviceName">@clientData.name</p>
                <p class="hunterDeviceOs">@string.Format("{0}({1})", clientData.uid, clientData.os)</p>
            </div>
            @*<DeviceListItem clientData="clientData" bSelected="@(clientData == curClientData)" OnClick="OnItemClick" />*@
        }
    </div>

    <div class="hunterContent">
        <div class="hunterContentHeader">
            @if (_curClientData != null)

            {
                <p class="hunterDeviceName moveUp">@_curClientData.name</p>
                <p class="hunterDeviceOs moveUp">@string.Format("{0}({1})", _curClientData.uid, _curClientData.os)</p>
             }
            </div>
        <div class="hunterContentData" id="scrollView">
            @*// <ol>*@
                @foreach (string message in this._messages)
                {
                    <div>@message</div>

                }
            @*</ol>*@
        </div>
    </div>
</div>
@code {
    private DeviceInfo _curClientData;
    private List<string> _messages = new List<string>();
    List<DeviceInfo> _clientDataList = new List<DeviceInfo>();
    HTMLDivElement scrollView;

    protected override async Task OnInitializedAsync()
    {
        await base.OnInitializedAsync();
        consoleHub.Connection.On<string>(HubMethodName.OnPrintLog, OnPrintLog);
        consoleHub.On<string[]>(HubMethodName.OnPrintLogs, OnPrintLogs);
        consoleHub.On<List<DeviceInfo>>(HubMethodName.OnQueryDevices, OnSyncDevices);
        consoleHub.On<string>(HubMethodName.OnDeviceDisconnected, OnDeviceDisconnected);
        consoleHub.Connection.On<int, string>(HubMethodName.OnListenToDevice, this.OnListendDevice);
        // 再请求下,服务器连接成功会主动推送,但是可能Console页面还没有初始化,没有注册回调接受数据
        if (consoleHub.IsConnected)
            await consoleHub.SendAsync(HubMethodName.QueryDevices);
    }

    protected override async Task OnAfterRenderAsync(bool firstRender)
    {
        await base.OnAfterRenderAsync(firstRender);
        if (scrollView == null)
            scrollView = Web.Document.GetElementById<HTMLDivElement>("scrollView");
        if (scrollView != null) // && _needScroll && _allowScroll)
        {
            scrollView.ScrollTo(0, scrollView.ScrollHeight);
        }
    }

    protected override void OnInitialized()
    {
        base.OnInitialized();
    }

    private async Task OnPrintLog(string message)
    {
        await Task.Run(() => this.PushMessage(message));
    }

    private void OnPrintLogs(string[] messages)
    {
        if (messages == null && messages.Length == 0)
            return;
        foreach (string message in messages)
        {
            this.PushMessage(message, false);
        }
        StateHasChanged();
    }

    private void PushMessage(string message, bool dirtyState = true)
    {
        // long startDate = System.DateTime.Now.Ticks / TimeSpan.TicksPerMillisecond;
        while (this._messages.Count >= 600)
        {
            this._messages.RemoveAt(0);
        }
        this._messages.Add(message);
        if (dirtyState)
            StateHasChanged();
        // long endDate = System.DateTime.Now.Ticks / TimeSpan.TicksPerMillisecond;
        // System.Console.WriteLine("------------------->PushMessage: {0}, cost: {1}", message, (endDate - startDate)/1000f);
    }

    void ListenToDevice(string uid)
    {
        if (_curClientData == null || _curClientData.uid != uid)
        {
            // await _hubConnection.SendAsync("ListeneToClient", Convert.ToString(clientData.id));
            consoleHub.ListenToDevice(uid);
            // this.PushMessage(MessageData.CreateSystemMessageData(string.Format("Listene To Client clientData.id = {0}", clientData.id)));
        }
    }

    private void OnDeviceDisconnected(string uid)
    {
        if (this._curClientData.uid == uid)
        {
            this._curClientData = null;
            StateHasChanged();
        }
    }


    private void OnListendDevice(int code, string deviceId)
    {
        if (code != 0)
            return;
        foreach (DeviceInfo deviceInfo in this._clientDataList)
        {
            if (deviceInfo.uid == deviceId)
                this._curClientData = deviceInfo;
        }
        // _curClientData = clientData;
        _messages.Clear();
        StateHasChanged();
    }


    private void OnSyncDevices(List<DeviceInfo> clientDataList)
    {
        _clientDataList = clientDataList;
        if (_curClientData != null)
        {
            foreach (var clientData in _clientDataList)
            {
                if (clientData.id == _curClientData.id)
                {
                    _curClientData = clientData;
                    break;
                }
            }
        }
        StateHasChanged();
    }
}

our app is used to show log, with the OnPrintLog callback, push the log message to web view

@SteveSandersonMS
Copy link
Member

SteveSandersonMS commented Jul 8, 2020

A few things immediately stand out:

  • Please consider using @key on all the lists in your render output. If you don't, the diffing system will have to do a lot of extra work to compare every single item deeply, and the DOM updates will be a lot more expensive. See docs here.
  • Why are you using Task.Run? This seems like it will just be harmful and not beneficial. When running on WebAssembly, this is single-threaded anyway, so doing Task.Run like this just creates a lot of extra work for every incoming log message.
  • If the "log messages" part of your UI updates much more frequently than the rest of the UI in this component, it's worth factoring out that piece into a separate child component that subscribes to message notifications independently. Then only that part of the UI has to re-render when a new log message arrives.
  • It looks like nothing ever unsubscribes from all the notifications. This will mean you have a memory leak if the user navigates away from this component. I'd recommend implementing IDisposable on your component, and in the Dispose method, unsubscribe from everything on consoleHub.

@SteveSandersonMS SteveSandersonMS added Needs: Author Feedback The author of this issue needs to respond in order for us to continue investigating this issue. and removed Needs: Attention 👋 This issue needs the attention of a contributor, typically because the OP has provided an update. labels Jul 8, 2020
@srxqds
Copy link
Author

srxqds commented Jul 8, 2020

thank you, @SteveSandersonMS I will try it right now.

@ghost ghost added Needs: Attention 👋 This issue needs the attention of a contributor, typically because the OP has provided an update. and removed Needs: Author Feedback The author of this issue needs to respond in order for us to continue investigating this issue. labels Jul 8, 2020
@srxqds
Copy link
Author

srxqds commented Jul 8, 2020

hi, @SteveSandersonMS here are the code with your advice, I remove unrelated code from it, this all the work code:

@page "/Console"

@using Hunter.App.Shared.Models
@using Hunter.App.Shared
@using Hunter.App.Client.Services
@using Microsoft.AspNetCore.SignalR.Client;
@using WebAssembly.Browser.DOM;
@inject NavigationManager NavigationManager
@inject ConsoleHubClient consoleHub

<div class="hunterContainer">
    <div class="hunterDevList">
        <p>请选择要跟踪的客户端</p>
        <hr />
        @foreach (var clientData in _clientDataList)
        {
            <div class="hunterDevice @((clientData == this._curClientData? " selected" : ""))" @onclick="@(e => ListenToDevice(clientData.uid))" @key="clientData">
                <p class="hunterDeviceName">@clientData.name</p>
                <p class="hunterDeviceOs">@string.Format("{0}({1})", clientData.uid, clientData.os)</p>
            </div>
        }
    </div>

    <div class="hunterContent">
        <div class="hunterContentHeader" @key="_curClientData">
            @if (_curClientData != null)
            {
                <p class="hunterDeviceName moveUp">@_curClientData.name</p>
                <p class="hunterDeviceOs moveUp">@string.Format("{0}({1})", _curClientData.uid, _curClientData.os)</p>
                
            }
        </div>
        <div class="hunterContentData" id="scrollView" @key="_messages">
            @foreach (MessageData message in this._messages)
            {
                <div @key="message">@message.content</div>
            }
        </div>
    </div>
</div>
@code {
    private DeviceInfo _curClientData;
    private List<MessageData> _messages = new List<MessageData>();
    private int _messageCount = 0;
    List<DeviceInfo> _clientDataList = new List<DeviceInfo>();
    HTMLDivElement scrollView;

    protected override async Task OnInitializedAsync()
    {
        await base.OnInitializedAsync();
        consoleHub.Connection.On<string>(HubMethodName.OnPrintLog, OnPrintLog);
        consoleHub.On<string[]>(HubMethodName.OnPrintLogs, OnPrintLogs);
        consoleHub.On<List<DeviceInfo>>(HubMethodName.OnQueryDevices, OnSyncDevices);
        consoleHub.On<string>(HubMethodName.OnDeviceDisconnected, OnDeviceDisconnected);
        consoleHub.Connection.On<int, string>(HubMethodName.OnListenToDevice, this.OnListendDevice);
        // 再请求下,服务器连接成功会主动推送,但是可能Console页面还没有初始化,没有注册回调接受数据
        if (consoleHub.IsConnected)
            await consoleHub.SendAsync(HubMethodName.QueryDevices);
    }

    protected override async Task OnAfterRenderAsync(bool firstRender)
    {
        await base.OnAfterRenderAsync(firstRender);
        if (scrollView == null)
            scrollView = Web.Document.GetElementById<HTMLDivElement>("scrollView");
        if (scrollView != null) // && _needScroll && _allowScroll)
        {
            scrollView.ScrollTo(0, scrollView.ScrollHeight);
        }
    }

    private void OnPrintLog(string message)
    {
        this.PushMessage(message);
    }

    private void PushMessage(string message, bool dirtyState = true)
    {
        while (this._messages.Count >= Hunter.Shared.Config.LogMessageMaxCount)
        {
            this._messages.RemoveAt(0);
        }
        this._messages.Add(MessageData.Create(message));
        if (dirtyState)
            StateHasChanged();
    }
}

I create div element always with @key, so it make no differnece with make child component of log messages.
The Console page only create one instance, so there is no effect of no unsubscribes from all the notifications.

but it also make ui freeze, as the performance of chrome DevTools it took a lot of time to execute script.

image

@srxqds
Copy link
Author

srxqds commented Jul 8, 2020

anyone can help me?

this is the call stack:
image

@SteveSandersonMS
Copy link
Member

@srxqds How fast do your log messages arrive? How many per second? Can you estimate the rate at which it becomes problematic?

@srxqds
Copy link
Author

srxqds commented Jul 8, 2020

@srxqds How fast do your log messages arrive? How many per second? Can you estimate the rate at which it becomes problematic?

I dispatch the message on server side 30 times per second, and only with 1000 line total message.

@SteveSandersonMS
Copy link
Member

SteveSandersonMS commented Jul 8, 2020

Re-rendering the whole log 30 times per second is going to put a lot of strain on the browser. It certainly can run that fast (on a reasonable laptop anyway) but I wouldn't do that for something that's meant to be left going in the background because it will still hurt the device's battery life.

Two techniques you can try to do this with much less CPU load are:

  1. Refresh the UI at a lower interval. Even if the messages are arriving 30 times per second, it won't make much difference to the user if you only re-render the UI showing the logs just 3 times per second (for example). Here's a self-contained example showing how you could reduce the render frequency using a timer.

  2. Instead of re-rendering the whole log in .razor logic each time, you could have Blazor just initially render an empty <div @ref="myElementReference"></div>. Then each time a log message arrives, use JS interop to pass myElementReference and newLogMessageText over to a JS function that appends the text to the div. Then no diffing will occur at all.

@SteveSandersonMS SteveSandersonMS added Needs: Author Feedback The author of this issue needs to respond in order for us to continue investigating this issue. and removed Needs: Attention 👋 This issue needs the attention of a contributor, typically because the OP has provided an update. labels Jul 8, 2020
@srxqds
Copy link
Author

srxqds commented Jul 9, 2020

Thank you @SteveSandersonMS, I use the timer up update view.
I can work with no ui freeze now but also with low frame rate.

@ghost ghost added Needs: Attention 👋 This issue needs the attention of a contributor, typically because the OP has provided an update. and removed Needs: Author Feedback The author of this issue needs to respond in order for us to continue investigating this issue. labels Jul 9, 2020
@SteveSandersonMS SteveSandersonMS added ✔️ Resolution: Answered Resolved because the question asked by the original author has been answered. and removed Needs: Attention 👋 This issue needs the attention of a contributor, typically because the OP has provided an update. labels Jul 9, 2020
@ghost ghost added the Status: Resolved label Jul 9, 2020
@srxqds srxqds closed this as completed Jul 9, 2020
@ghost ghost locked as resolved and limited conversation to collaborators Aug 9, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-blazor Includes: Blazor, Razor Components feature-blazor-wasm This issue is related to and / or impacts Blazor WebAssembly ✔️ Resolution: Answered Resolved because the question asked by the original author has been answered. Status: Resolved
Projects
None yet
Development

No branches or pull requests

2 participants