4

I have an activity, which contains 2 fragments, each fragment has 2 list views, each has an adapter. So I get pretty long data in json from the server, I use AsyncTask and on post execute, I parse the data. The problem is that it freezes the loading animation for 4-5 seconds.

I have tried to use Handler thread, but the problem is still there, I must be doing something terribly wrong here.

public class DataService extends AsyncTask<String, String, Void> {

    @Override
        protected Void doInBackground(String... params) {

        url = new URL(DATA_URL);    
        //Connection stuff

        String jsonString = stringBuilder.toString();
        jsonObject = new JSONObject(jsonString);

        //Adding data in Java Objects

    }


    @Override
    protected void onPostExecute(Void aVoid) {
            super.onPostExecute(aVoid);


             //Upcoming List in Fragment
             allDataFragment.setUpcomingDataList(UpcomingAllDataList);
             awayFragment.setUpcomingDataList(tUpcomingAwayList);
             homeFragment.setUpcomingDataList(tUpcomingHomeList);


             //Past List in Fragment            
             allDataFragment.setPastDataList(pastAllDataList);
             awayFragment.setPastDataList(pastAwayList);
             homeFragment.setPastDataList(pastHomeList);    

    }

I added log messages in adapter, and I can see that at the time of parsing rows it freezes, so it take all the post execute stuff in handler thread

       Handler handler= new Handler();

        handler.post(new Runnable() {
            @Override
            public void run() {


                    //Upcoming List in Fragment
                     allDataFragment.setUpcomingDataList(UpcomingAllDataList);
                     awayFragment.setUpcomingDataList(tUpcomingAwayList);
                     homeFragment.setUpcomingDataList(tUpcomingHomeList);


                     //Past List in Fragment            
                     allDataFragment.setPastDataList(pastAllDataList);
                     awayFragment.setPastDataList(pastAwayList);
                     homeFragment.setPastDataList(pastHomeList);    


            }                   
            });

I also tried to add a handler in Adapter but it fails to load UI component from there.

This is a snippet from my fragment code.

if (pastListView != null) {
        PastListAdapter allGamesAdapter = new PastListAdapter(getContext(), pastAllDataList);
        pastListView.setAdapter(allGamesAdapter);
        }

and in Adapter i am doing following.

  @Override
    public View getView(int position, View convertView, ViewGroup parent) {

            TextView vScore = (TextView) v.findViewById(R.id.v_score);
            TextView date = (TextView) v.findViewById(R.id.date);
            ImageView image = (ImageView) v.findViewById(R.id.iv_image);
            //7 Other views


            vScore.setText(dataList.get(position).getScore);
            date.setText(dataList.get(position).date);

             Log.d(TAG, "getView: Row");

            return v;
    }

The loading animation works fine in start but at the time it starts parsing the data in adapter, it hangs t he UI thread.

Farzad Karimi
  • 770
  • 1
  • 12
  • 31
dev90
  • 7,187
  • 15
  • 80
  • 153
  • 4
    *so it take all the post execute stuff in handler thread* ... Handler is created on UI thread so "handler thread" == UI thread ... so it obviosuly will not help ... you should mesure what part of the program takes a long time and then fix it ... problems with `getView()` one obvious reasons: you are not using recycling ... one "prolly" reason(but without code it's just blind guess) you are loading images on UI thread ... – Selvin Oct 11 '17 at 08:24
  • @Selvin I am not using `RecyclerView` what i see is that `getView()` is taking time, but i am unable to understand how to fix it, getView() should be in UI thread, I am unable to find any way to put that part in some other thread – dev90 Oct 11 '17 at 08:38
  • You are using ListView as Adapter for RecyclerView has no getView method... Also you have to find out what exactly in getView takes so much time. I bet on loading images which should be done on background thread. – Selvin Oct 11 '17 at 08:49
  • And, yes, ListView supports view recycling ... convertView is recycled view - it is null when you need create new object and not null if view can be reused – Selvin Oct 11 '17 at 09:19
  • @Selvin : I have comment out the Image loading part, It still hangs for few seconds, what i can see at the time of rendering `rows` it hangs for few seconds.. I have added Log message, that prints on each row `getView` How can i move `Base Adapter - getView` to some other thread? – dev90 Oct 11 '17 at 09:28
  • You can't... Creation of the view a should be done on his thread. also(another blind guessing) the problem can be if you put ListView into another scrollable view and use very bad solution(create and mesure all items height) to make it scroll – Selvin Oct 11 '17 at 09:39
  • @Selvin whats the solution then :( There 94 rows, so `getView` runs 94 times, and gets hanged for few seconds – dev90 Oct 11 '17 at 09:44
  • The solution is: learn to debug and profile your code – Selvin Oct 11 '17 at 09:46
  • @Kirmani88 you say "there are 94 rows, so `getView` runs 94 times". This shouldn't happen unless all rows are visible at the same time. Is your ListView inside a different scrolling container or something like that? – Ben P. Oct 18 '17 at 18:56

5 Answers5

2

Use AsyncTask to handle work items shorter than 5ms ( 5 milli seconds) in duration. Since you are getting pretty long JSON data from server, you have to look at other alternatives ( Thread, HandlerThread and ThreadPoolExecutor)

Even though you create Handler, it's running on UI Thread only. Instead of Handler, use HandlerThread

Proposed solution:

  1. Use HandlerThread to get JSON data from server. UI Thread is not blocked to get the data.

  2. Post Data from HandlerThread to Handler of UI Thread. UI Handler will take care of updating UI with data received on handleMessage(Message msg)

Useful posts:

Asynctask vs Thread vs Services vs Loader

Handler vs AsyncTask vs Thread

Why to use Handlers while runOnUiThread does the same? ( This post contains code example for above proposed solution)

Ravindra babu
  • 37,698
  • 11
  • 250
  • 211
2

Use Volley instead of AsyncTask

1

If it's blocking the ui thread then don't do it in the ui thread

You already got an asyncTask so just use it and call your parsing json function from there

Then you can return void from that task and update your adopter from there

In other words, move your logic from onPostExecute to doInBackground

Moshe Edri
  • 244
  • 2
  • 10
  • Array Adapter `getView()` is causing the problem, If i move `fragment.setDataList` from `postExecute` to `doInBackground` then background thread cant access the UI objects – dev90 Oct 11 '17 at 08:39
  • Well then write a function and call it from the task That should do it – Moshe Edri Oct 11 '17 at 09:09
  • I don't understand your use of this data but it looks like it should be simply or divided and query in a few api call How big is your data? Maybe consider pagination – Moshe Edri Oct 11 '17 at 09:17
1

at the beginning, i advise you, not be a lazy, and look to libraries

Retrofit - RESTfull client library

OKhttp - http client library

You do not need to write AsyncTasks with these libraries, and this will make your life easier and painless.

But, if you won't, ok. Let's talk about AsyncTask problems and decisions of your concerns.

AsyncTask are runs on a background computation thread and whose result is published on the UI thread. They are defined by 3 generic types, called Params, Progress and Result, and 4 steps, called begin, doInBackground, progress updating and end of computations with updating UI. So, when it is executed, the task goes through 4 steps:

1 step. onPreExecute(), invoked on the UI thread immediately after the task is executed. This step is basically used to configure the task, for example showing a progress bar, progress dialog or any other changes in the UI.

2 step. doInBackground(Params...), invoked on the background thread immediately after onPreExecute() finishes executing. This step is used to perform background computation that can take a long time. The parameters of the asynchronous task are passed to this step. The result of the computation must be returned by this step and will be passed back to the last step. This step can also use publishProgress(Progress...) to publish one or more units of progress. These values are published on the UI thread, in the onProgressUpdate(Progress...) step.

3 step. onProgressUpdate(Progress...), invoked on the UI thread after a call to publishProgress(Progress...). The timing of the execution is undefined. This method is used to display any form of progress in the user interface while the background computation is still executing. For instance, it can be used to animate a progress bar or show logs in a text field.

4 step. onPostExecute(Result), invoked on the UI thread after the background computation finishes. The result of the background computation is passed to this step as a parameter.

So, as you also now, if you don't want catch performance freezing, you have to return result of the computation in 2 step.

public class DataService extends AsyncTask<String, String, List<UpcomingAllDataList>> {

    @Override
    protected List<UpcomingAllDataList> doInBackground(String... params) {

        // making HTTP request

        return UpcomingAllDataList; // return results of Http request
    }

    @Override
    protected void onPostExecute(List<UpcomingAllDataList> upcomingAllDataList) {
        super.onPostExecute(upcomingAllDataList);


        //Upcoming List in Fragment
        allDataFragment.setUpcomingDataList(upcomingAllDataList);

        // all another work

    }
}

Hope, it will be usefull.

Scrobot
  • 1,911
  • 3
  • 19
  • 36
0

you should really use Retrofit for the server call + parsing.
Retrofit Github

Retrofit Site

I think the real problem is that you parse more data in setUpcomingDataList (and all the others) methods.

anyway, your example for HandlerThread is actually happening in the UI thread since Handler's default constructor takes the current thread that you construct the Handler on (which in your case is the UI thread) and post the Runnable to that thread.

anyway 2 - to see if it the methods would really work in a different thread (i guess you will get an android.view.ViewRoot$CalledFromWrongThreadException: Only the original thread that created a view hierarchy can touch its views. )
you can try to wrap your code with normal thread:

Thread thread = new Thread(new Runnable() {
                @Override
                public void run() {
                    //Upcoming List in Fragment
                    allDataFragment.setUpcomingDataList(UpcomingAllDataList);
                    awayFragment.setUpcomingDataList(tUpcomingAwayList);
                    homeFragment.setUpcomingDataList(tUpcomingHomeList);


                    //Past List in Fragment            
                    allDataFragment.setPastDataList(pastAllDataList);
                    awayFragment.setPastDataList(pastAwayList);
                    homeFragment.setPastDataList(pastHomeList);
                }
            });
            thread.start();

which is a really bad practice - so as I said before, use Retrofit.

Farzad Karimi
  • 770
  • 1
  • 12
  • 31
Alon Aviram
  • 190
  • 1
  • 15
  • I think, it's not good decision... for updating UI Thread better use getActivity().runOnUiThread(() -> {}) – Scrobot Oct 21 '17 at 07:28