WinRT - Loading data while keeping the UI responsive

Question

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";
        }
    }

Answer 1

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.

Answer 2

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.

Answer 3

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.

Answer 4

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)?

Answer 5

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.