1
\$\begingroup\$

I am using the following class to retrieve a http response, which contains data for my Android App. Everything works fine, but there is a performance issue. I get the following warning when requesting this class:

Skipped 36 frames! The application may be doing too much work on its main thread.

I used an AsyncTask to run the network connection on a different thread, but it is still saying I'm doing too much work on the main thread. Is there something you can point out to me that is slowing down my program?

package com.lakshan.gamematch.util;

import java.io.BufferedReader;
import java.io.InputStream;
import java.io.InputStreamReader;

import org.apache.http.HttpEntity;
import org.apache.http.HttpResponse;
import org.apache.http.client.HttpClient;
import org.apache.http.client.methods.HttpPost;
import org.apache.http.impl.client.DefaultHttpClient;

import android.os.AsyncTask;
import android.util.Log;

public class HttpRequest {

String url_to_task = "";
String string_result = "hasn't returned a value";

        public String getFromURL (String url) {
            url_to_task = url;
            new MyTask().execute();
            while (string_result.equals("hasn't returned a value")) {
                try {
                    Thread.sleep(10);
                } catch (InterruptedException e) {
                    e.printStackTrace();
                }
            }
            return string_result;
        }

        private class MyTask extends AsyncTask <Void, Void, Void> {
            @Override
            protected Void doInBackground(Void... params) {
                InputStream is = null;

                //Download JSON data from url
                try {
                    HttpClient httpclient = new DefaultHttpClient();
                    HttpPost httppost = new HttpPost(url_to_task);
                    HttpResponse response = httpclient.execute(httppost);
                    HttpEntity entity = response.getEntity();
                    is = entity.getContent();
                    Log.e("entity", entity.getContent().toString());

                } catch (Exception e) {
                    Log.e("log_tag", "Error in http connection " + e.toString());
                }

                //Convert response to string
                try {
                    BufferedReader reader = new BufferedReader (new InputStreamReader(is, "iso-8859-1"), 8);
                    StringBuilder sb = new StringBuilder();
                    String line = null;
                    while ((line = reader.readLine()) != null) {
                        sb.append(line + "\n");

                    }
                    is.close();
                    string_result = sb.toString();
                } catch (Exception e) {
                    Log.e("log_tag", "Error converting result" + e.toString());
                }
                return null;
            }

            @Override
            protected void onPostExecute(Void result) {             
                super.onPostExecute(result);
            }
        }
}
\$\endgroup\$
  • \$\begingroup\$ In my opinion you should try to use Retrofit2 library with Scalars Converter Factory. It will MUCH simplify your code and you won't have to use AsyncTask then. \$\endgroup\$ – Pawel Urban Oct 4 '16 at 17:32
3
\$\begingroup\$
  1. By default AsyncTask tasks execute sirially on the default executor and you should at least override the execute to make sure that all requests are run in parallel unless specified otherwise

  2. Apache HTTP library is considered deprecated in Android and you should try to at least use HttpUrlConnection if not a library that can make this happen with zero boilerplate code sych as Volley, OkHttp or Retrofit

  3. Using Thread.sleep on the main thread is the best way you have to cause these warnings.
    The system has about 16.8 milliseconds to draw anything on the screen and you just took away 10 of them for nothing while blocking the system from using the thread.
    please use the given onPreExecute and onPostExecute to show progress activity to the user in any way you see fit and never EVER sleep on the main thread

\$\endgroup\$
  • \$\begingroup\$ Point 1 is wrong. The default executor is a serial one. They experimented with a default parallel executor (from Donut to Gingerbread) but reverted to serial execution starting with Honeycomb. I upvoted your answer anyway because point 3 correctly addresses the OP's question. \$\endgroup\$ – Ivanhoe Oct 10 '16 at 11:48
  • \$\begingroup\$ edited, had a brain fart and wrote parallel instead of sirial \$\endgroup\$ – thepoosh Oct 10 '16 at 14:51
  • \$\begingroup\$ Thank you for your answer, pointed me in the right direction! I am going to give it a try now. \$\endgroup\$ – Lakshan Sivananthan Oct 11 '16 at 18:39

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.