6

I'm developing a Windows Metro app, and am getting an issue with the UI becoming unresponsive. As far as I can tell, the cause is as follows:

    <ListView
...
        SelectionChanged="ItemListView_SelectionChanged"            
...

This event is handled here:

    async void ItemListView_SelectionChanged(object sender, SelectionChangedEventArgs e)
    {
        if (this.UsingLogicalPageNavigation()) this.InvalidateVisualState();

        MyDataItem dataItem = e.AddedItems[0] as MyDataItem;
        await LoadMyPage(dataItem);
    }

    private async Task LoadMyPage(MyDataItem dataItem)
    {            
        SyndicationClient client = new SyndicationClient();
        SyndicationFeed feed = await client.RetrieveFeedAsync(new Uri(FEED_URI));                    

        string html = ConvertRSSToHtml(feed)
        myWebView.NavigateToString(html, true);            
    }

LoadMyPage takes a while to complete, as it gets data from a web service and loads it onto the screen. However, it looks like the UI is waiting for it: my guess is that until the above event completes.

So my question is: what can I do about this? Is there a better event I can hook into, or is there another way to handle this? I thought about starting a background task, but that seems like overkill to me.

EDIT:

Just to clarify the scale of this problem, I'm talking about a maximum of 3 - 4 seconds unresponsive. This is by no means a long running job.

EDIT:

I've tried some of the suggestion below, however, the entire call stack from the SelectionChanged function is using async/await. I've tracked it down to this statement:

myFeed = await client.RetrieveFeedAsync(uri);

Which doesn't seem to be continuing processing until it's complete.

EDIT:

I realise this is turning into War & Peace, but below is a replication of the problem using a blank metro app and a button:

XAML:

<Grid Background="{StaticResource ApplicationPageBackgroundThemeBrush}">
    <StackPanel>
        <Button Click="Button_Click_1" Width="200" Height="200">test</Button>
        <TextBlock x:Name="test"/>
    </StackPanel>
</Grid>

Code behind:

    private async void Button_Click_1(object sender, RoutedEventArgs e)
    {
        SyndicationFeed feed = null;

        SyndicationClient client = new SyndicationClient();
        Uri feedUri = new Uri(myUri);

        try
        {
            feed = await client.RetrieveFeedAsync(feedUri);

            foreach (var item in feed.Items)
            {       
                test.Text += item.Summary.Text + Environment.NewLine;                    
            }
        }
        catch
        {
            test.Text += "Connection failed\n";
        }
    }


  • What do you mean by "Unresponsive"? If you place a button on the page and try to mouse over, do you see the mouse over behavior? Can you interact with the page at all? Are you trying to transition the page during LoadPage? If the RetrieveFeedAsync method takes awhile to complete, it will not lock the UI, but it will seem as if nothing is happening. - Shawn Kendrot
  • The controls on the screen do not respond. The mouse moves, but the UI of the app is temporarily not usable. - pm_2
  • How many items are in the returned feed? - Stephen Cleary
  • In the example above, there's 50... but I get the same behaviour if I remove the foreach loop altogether - pm_2
  • I can't see the code for LoadMyPage anywhere... please include that. Ideally, please strip this down to a short but complete program which demonstrates the problem. - Jon Skeet

5 답변


5

Give this a try...

SyndicationFeed feed = null;

SyndicationClient client = new SyndicationClient();

var feedUri = new Uri(myUri);

try {
    var task = client.RetrieveFeedAsync(feedUri).AsTask();

    task.ContinueWith((x) => {
        var result = x.Result;

        Parallel.ForEach(result.Items, item => {
            Dispatcher.RunAsync(Windows.UI.Core.CoreDispatcherPriority.Normal,
            () =>
            {
                test.Text += item.Title.Text;
            });
       });     
   });
}
catch (Exception ex) { }

I tried it on my machine by adding a button to an app using the Grid app template. I could scroll the grid of items back and forth while I updated the title of the page without problem. Though I didn't have a lot of items to it went really fast so it was tough to be 100% positive.


  • This works (I left our the Parallel ForEach) - thanks. I was under the impression that await effectively did the same as this - so what does this do that await doesn't? - pm_2
  • When you use await, the compiler will automatically marshal the code that follows the completion of the async call back onto the UI thread. Tasks are used in the background to accomplish this, but the compiler magic moves the opportunity for background processing back onto the main thread. In my code, we take control of the task explicitly and use it to continue with work on its own thread instead of going back to the UI thread. Depending on how much data your looping through and how you want the UI to update, you may want to stick with the Parallel.ForEach - Jeff Brand
  • You're leaking exceptions there, aren't you? The task is quick to start and the continuation is not in the same synchronization context that you have exception handling on. - Henrik

4

Since you are using await in front of LoadMyPage I am assuming that it compiles and that it returns a Task. Given that, I've created a little example.

Let us assume that LoadMyPage(and Sleep()) looks like this:

public Task<string> LoadMyPage()
{
    return Task<string>.Factory.StartNew(() =>
                                                {
                                                    Sleep(3000);
                                                    return "Hello world";
                                                });
}
static void Sleep(int ms)
{
    new ManualResetEvent(false).WaitOne(ms);
}

And that the XAML looks like this:

<StackPanel>
    <TextBlock x:Name="Result" />
    <ListView x:Name="MyList" SelectionChanged="ItemListView_SelectionChanged">
        <ListViewItem>Test</ListViewItem>
        <ListViewItem>Test2</ListViewItem>
    </ListView>
    <Button>Some Button</Button>
    <Button>Some Button2</Button>
</StackPanel>

We can then have the SelectionChanged event handler looking like this:

private async void ItemListView_SelectionChanged(object sender,
                                                 SelectionChangedEventArgs e)
{
    MyList.IsEnabled = false;
    var result = await LoadMyPage();

    Result.Text = result;

    MyList.IsEnabled = true;
}

The Task that LoadMyPage returns will run in parallel which means that when that task is running, the UI should not freeze. Now to get the result from that Task you use await. This will create a continuation block.

So in this example, when you select something, the ListView is disabled for the entire loading time and then re-enabled once the Task has finished. You can verify that the UI didn't freeze up by pressing the buttons to see that it is still responsive.

If LoadMyPage interacts with the UI, you need to re-arrange it a little bit, have it return a ViewModel or the result that you want and then put everything together again on the UI thread.


  • Unfortunately I can't do that because I'm using the asynchronous method RetrieveFeedAsync - pm_2
  • -1: the question is about async, not about long running items and you can't sleep in WinRT. - Henrik
  • @Henrik, Read my answer and check the code samples again. I'm simulating Sleep by using ManualResetEvent(false).WaitOne(ms). I don't see how my answer is not about async, could you please clearify? - Filip Ekberg
  • My point is that you are blocking your thread. - Henrik
  • @Henrik, No? The Task runs asynchronously and is awaited in ItemListView_SelectionChanged. - Filip Ekberg

2

A background thread is most definitely not overkill. That's precisely how you handle this kind of problem.

Don't perform lengthy tasks on the UI thread, or you will tie up the UI and cause it to become unresponsive. Run these on a background thread, and then have that thread raise an event that can be processed by the main UI thread when it finishes.

Showing some kind of progress indicator on the UI thread is also useful. Users like to know that something is happening. That will reassure them that the app isn't broken or frozen, and they'll be willing to wait a bit longer. This is why all web browsers have some kind of "throbber" or other loading indicator.


2

The most likely problem is that LoadMyPage is doing something synchronously. Remember, async doesn't run your code on a background thread; by default all of its actual code will be run on the UI thread (see the async/await FAQ or my async/await intro). So, if you block in an asynchronous method, it's still blocking the calling thread.

Take a look at LoadMyPage. Is it using await to call the web service? Is it doing expensive processing of the data before putting it in the UI? Is it overwhelming the UI (many Windows controls have scalability problems when they get to thousands of elements)?


  • See latest edit - the most expensive thing it's doing is a call to the web. It takes between 1 and 5 secs, but is stalling the UI while it's there. - pm_2
  • Try this test: change LoadMyPage to call await Task.Run(() => {}).ConfigureAwait(false) before it calls RetrieveFeedAsync. What happens? - Stephen Cleary
  • Makes no difference whatsoever - pm_2
  • Please post all the code needed to reproduce this. - Stephen Cleary
  • See above edits - pm_2

2

Looking at your simplified code example, I believe your problem is everything that sits outside the await line.

In the following block of code:

private async void Button_Click_1(object sender, RoutedEventArgs e)
    {
        SyndicationFeed feed = null;

        SyndicationClient client = new SyndicationClient();
        Uri feedUri = new Uri(myUri);

        try
        {
            feed = await client.RetrieveFeedAsync(feedUri);

            foreach (var item in feed.Items)
            {       
                test.Text += item.Summary.Text + Environment.NewLine;                    
            }
        }
        catch
        {
            test.Text += "Connection failed\n";
        }
    }

The only line that is executing on a background thread is the line

feed = await client.RetrieveFeedAsync(feedUri);

All other lines of code in that block are executing on the UI thread.

Just because your button click handler is marked as async doesn't mean code within it doesn't run on the UI thread. In fact the event handler starts ON the UI thread. So, creating the SyndicationClient and setting the Uri happens on the UI thread.

Something many developers don't realize is that any code that comes after await will automatically resume on the same thread that was in use before the await. This means the code

            foreach (var item in feed.Items)
            {       
                test.Text += item.Summary.Text + Environment.NewLine;                    
            }

is running on the UI thread!

This is handy in that you don't have to do Dispatcher.Invoke to update test.Text, but it also means you're blocking the UI thread the whole time you're looping through items and concatenating strings.

In your (albeit simplified) example, the easiest way to do this work on the background thread would be to have another method on SyndicationClient called RetrieveFeedAsStringAsync; Then SyndicationClient can do the download, looping and concatenation of strings all as part of its own task. After that task completes, the only line of code that would run on the UI thread would be assigning the text to the TextBox.


  • But I get the same behaviour if I completely remove the foreach() loop. - pm_2

Linked


Related

Latest