Android RSS parsing too slow - doing something wrong?

Question

I followed a tutorial on how to parse XML(RSS) files on an Android app. Below is my code that is working, but is taking quite long. Am I doing something incorrect or in a way that would slow it down? It takes 7-10 seconds to load 30-35 articles... seemingly way too long, and certainly too long for a user to wait every time they switch sections (which pulls in another feed)

I do have things in place to only pull new data every hour...etc ... but the first time still takes 10 seconds usually, which - by then, the user thinks their app has crashed...etc.

• I am doing this in an AsyncTask
• I'm not downloading the images at this point
• This IS the slow part of the code (not internet, or displaying... etc)

TLDR:
This code is too slow - why?:

package com.mysite.utilities;

import java.util.ArrayList;

import org.xml.sax.Attributes;
import org.xml.sax.helpers.DefaultHandler;

public class RSSHandler extends DefaultHandler
{

    // creats a news item class to hold data within loop
    public class NewsItem
    {
        public String title;
        public String subhead;
        public String link;
        public String byline;
        public String description;
        public String storytext;
        public String cmsstoryid;
        public String pubDate;
        public String keyword;
        public String category;
        public String subcategory;
        public String slotreference;
        public String lastModTime;
        public ArrayList<PhotoItem> photos = new ArrayList<PhotoItem>();

        @Override
        public String toString()
        {
            return title;
        }
    }

    //creates a photo item class to hold image data within loop
    public class PhotoItem
    {
        public String photoid; //tribs id
        public String caption;
        public String height;
        public String width;
        public String photo; //full path to photo
        public String thumbnail; //full path to thumbnail
        public int articleid;
        public String photolastmodified;

        @Override
        public String toString()
        {
            return photo;
        }
    }

    //creates instances of classes to be used in loop
    private StringBuffer buf; //to hold element characters with loop
    private ArrayList<NewsItem> feedItems;
    private NewsItem item;
    private PhotoItem photoitem;

    //initialize variables of whether or not it's within an item (or photo item)
    private boolean inItem = false;
    private boolean inPhotoItem = false;

    //
    public ArrayList<NewsItem> getParsedItems() {
        return feedItems;
    }

    //Called at the head of each new element
    @Override
    public void startElement(String uri, String name, String qName, Attributes atts)
    {
        //creates an array of news items
        if ("channel".equals(name))
        {
            feedItems = new ArrayList<NewsItem>();
        }
        //creates a news item and toggles "in news item" to on
        else if ("item".equals(name))
        {
            item = new NewsItem();
            inItem = true;
        }
        //creates a photo item and toggles "in photo item" to on
        else if ("image".equals(name))
        {
            photoitem = new PhotoItem();
            inPhotoItem = true;
        }
        //starts a new string buffer if matches an elemnt we want from news item
        else if (
            ("title".equals(name) ||
            "subhead".equals(name) ||
            "link".equals(name) ||
            "byline".equals(name) ||
            "description".equals(name) ||
            "storytext".equals(name) ||
            "keyword".equals(name) ||
            "cmsstoryid".equals(name) ||
            "pubDate".equals(name) ||
            "category".equals(name) ||
            "subcategory".equals(name) ||
            "lastModTime".equals(name) ||
            "slotreference".equals(name)
            )
            && inItem)
        {
            buf = new StringBuffer();
        }
        //starts an a new string buffer if it matches an element we want from image item
        else if(
            ("caption".equals(name) ||
            "photoid".equals(name) ||
            "height".equals(name) ||
            "width".equals(name) ||
            "photo".equals(name) ||
            "photolastmodified".equals(name) ||
            "thumbnail".equals(name)
            )
            && inPhotoItem)
        {
            buf = new StringBuffer();
        }
    }

    //Called at the tail of each element end
    @Override
    public void endElement(String uri, String name, String qName)
    {       
        if ("item".equals(name))
        {
            feedItems.add(item);
            inItem = false;
        }
        else if ("image".equals(name))
        {
            try {
                item.photos.add(photoitem);
            } catch (Exception e) {
                System.out.println(e);
            }
            inPhotoItem = false;
        }
        else if (inItem)
        {
            if (inPhotoItem)
            {
                if ("caption".equals(name)) { photoitem.caption = buf.toString(); }
                else if ("photoid".equals(name)) { photoitem.photoid = buf.toString(); }
                else if ("height".equals(name)) { photoitem.height = buf.toString(); }
                else if ("width".equals(name)) { photoitem.width = buf.toString(); }
                else if ("photo".equals(name)) { photoitem.photo = buf.toString(); }
                else if ("photolastmodified".equals(name)) { photoitem.photolastmodified = buf.toString(); }
                else if ("thumbnail".equals(name)) { photoitem.thumbnail = buf.toString(); }
            }
            else if ("title".equals(name)) { item.title = buf.toString(); }
            else if ("subhead".equals(name)) { item.subhead = buf.toString(); }
            else if ("link".equals(name)) { item.link = buf.toString(); }
            else if ("byline".equals(name)) { item.byline = buf.toString(); }
            else if ("description".equals(name)) { item.description = buf.toString(); }
            else if ("storytext".equals(name)) { item.storytext = buf.toString(); }
            else if ("keyword".equals(name)) { item.keyword = buf.toString(); }
            else if ("cmsstoryid".equals(name)) { item.cmsstoryid = buf.toString(); }
            else if ("pubDate".equals(name)) { item.pubDate = buf.toString(); }
            else if ("category".equals(name)) { item.category = buf.toString(); }
            else if ("subcategory".equals(name)) { item.subcategory = buf.toString(); }
            else if ("lastModTime".equals(name)) { item.lastModTime = buf.toString(); }
            else if ("slotreference".equals(name)) { item.slotreference = buf.toString(); }

        }
        else
        {
            buf = null;
        }

    }

    //Called with character data inside elements
    @Override
    public void characters(char ch[], int start, int length)
    {
        //Don't bother if buffer isn't initialized
        if(buf != null)
        {
            for (int i=start; i<start+length; i++)
            {
                buf.append(ch[i]);
            }
        }
    }
}

---

My updated code thus far (still no speed increase):

package com.sltrib.utilities;

import java.util.ArrayList;

import org.xml.sax.Attributes;
import org.xml.sax.helpers.DefaultHandler;

import android.util.Log;

public class RSSHandler extends DefaultHandler
{

    // creats a news item class to hold data within loop
    public class NewsItem
    {
        public String title;
        public String subhead;
        public String link;
        public String byline;
        public String description;
        public String storytext;
        public String cmsstoryid;
        public String pubDate;
        public String keyword;
        public String category;
        public String subcategory;
        public String slotreference;
        public String lastModTime;
        public ArrayList<PhotoItem> photos = new ArrayList<PhotoItem>();

        @Override
        public String toString()
        {
            return title;
        }
    }

    //creates a photo item class to hold image data within loop
    public class PhotoItem
    {
        public String photoid; //tribs id
        public String caption;
        public String height;
        public String width;
        public String photo; //full path to photo
        public String thumbnail; //full path to thumbnail
        public int articleid;
        public String photolastmodified;

        @Override
        public String toString()
        {
            return photo;
        }
    }

    //creates instances of classes to be used in loop
    private StringBuilder buf; //to hold element characters with loop
    private ArrayList<NewsItem> feedItems;
    private NewsItem item;
    private PhotoItem photoitem;

    //initialize variables of whether or not it's within an item (or photo item)
    private boolean inItem = false;

    private int currentTagId = 0;

    //
    public ArrayList<NewsItem> getParsedItems() {
        return feedItems;
    }


    @Override
    public void startDocument()
    {
        feedItems = new ArrayList<NewsItem>(); //creates an array to hold feed items
    }


    //Called at the head of each new element
    @Override
    public void startElement(String uri, String name, String qName, Attributes atts)
    {
        //gets the id of the current tag
        currentTagId = 0;
        String tmpId = atts.getValue("id");
        if(tmpId != null)
        {
            currentTagId = Integer.parseInt(tmpId);
        }

        //creates a news item
        if (currentTagId == 99) // <item>
        {
            item = new NewsItem();
            inItem = true;
        }

        //creates a photo item
        else if (currentTagId == 21) { // <image>
            photoitem = new PhotoItem();
        }

        //creates a string builder if it's a tag within an article or a photo
        else if (inItem && currentTagId > 0 && currentTagId < 29)
        {
            buf = new StringBuilder();
        }
    }


    //Called at the tail of each element end
    @Override
    public void endElement(String uri, String name, String qName)
    {

        // it's not getting the tag id - how?

        Log.d("XML", "endElement:" + Integer.toString(currentTagId));

        if (currentTagId == 99)  // </item>
        {
            Log.d("XML", "Should be adding item: " + item.title);
            feedItems.add(item);
            inItem=false;
        }

        else if (currentTagId == 21) // </image>
        {
            try {
                item.photos.add(photoitem);
            } catch (Exception e) {
                System.out.println(e);
            }
        }
        else
        {
            switch(currentTagId)
            {
                case 1: item.title = buf.toString(); break;
                case 2: item.subhead = buf.toString(); break;
                case 3: item.link = buf.toString(); break;
                case 4: item.byline = buf.toString(); break;
                case 5: item.description = buf.toString(); break;
                case 6: item.storytext = buf.toString(); break;
                case 11: item.cmsstoryid = buf.toString(); break;
                case 14: item.pubDate = buf.toString(); break;
                case 15: item.lastModTime = buf.toString(); break;
                case 16: item.keyword = buf.toString(); break;
                case 17: item.category = buf.toString(); break;
                case 18: item.subcategory = buf.toString(); break;
                case 19: item.slotreference = buf.toString(); break;
                case 22: photoitem.caption = buf.toString(); break;
                case 23: photoitem.photoid = buf.toString(); break;
                case 24: photoitem.photolastmodified = buf.toString(); break;
                case 25: photoitem.height = buf.toString(); break;
                case 26: photoitem.width = buf.toString(); break;
                case 27: photoitem.photo = buf.toString(); break;
                case 28: photoitem.thumbnail = buf.toString(); break;
                default: if(!inItem) buf = null;
            }
        }
    }

    //Called with character data inside elements
    @Override
    public void characters(char ch[], int start, int length)
    {
        if(buf != null)
        {
            String chars = new String(ch, start, length); // get all text value inside the element tag
            chars = chars.trim(); // remove all white-space characters
            buf.append(chars);
        }
    }
}

Answer 1

Perhaps you are already doing this, but I just wanted to reiterate that this should be done on another thread. The user wont think the app has crashed if you display a progress indicator (spinner) and keep the GUI responsive.

If you are not doing this on a background thread, it could be making this artificially slow because the system is fighting to keep the user interface going.

As to why this is slow, there is a TON of branching. The more if statements you have, and the more elses (etc) you have, the more difficult it is for the branch predictor do do its job.

Also, I can't tell if you are downloading the images here, but if you are, that should be done asynchronously after the text data is retrieved and displayed to the user (to make the user interface more responsive).