views:

1998

answers:

5

Hey,

I am making an HTTP get request to a website for an android application I am making.

I am using a DefaultHttpClient and using HttpGet to issue the request. I get the entity response and from this obtain an InputStream object for getting the html of the page.

I then cycle through the reply doing as follows:

BufferedReader r = new BufferedReader(new InputStreamReader(inputStream));
String x = "";
x = r.readLine();
String total = "";

while(x!= null){
total += x;
x = r.readLine();
}

However this is horrendously slow.

Is this inefficient? I'm not loading a big web page - www.cokezone.co.uk so the file size is not big. Is there a better way to do this?

Thanks

Andy

A: 

If the file is long, you can optimize your code by appending to a StringBuilder instead of using a String concatenation for each line.

Maurice Perry
Its not that long to be honest - its the page source of the website www.cokezone.co.uk - so really not that big. Definitely less than 100kb.
RenegadeAndy
Does anybody have any other ideas on how this could be made more efficient - or if this is even inefficient!? If the latter is true - why does it take SO long? I dont believe the connection is to blame.
RenegadeAndy
A: 

maybe rather then read 'one line at a time' and join the strings, try 'read all available' so as to avoid the scanning for end of line, and to also avoid string joins.

ie, InputStream.available() and InputStream.read(byte[] b, int offset, int length)

steelbytes
Hmm. so it would be like this:int offset = 5000;Byte[] bArr = new Byte[100];Byte[] total = Byte[5000];while(InputStream.available){offset = InputStream.read(bArr,offset,100);for(int i=0;i<offset;i++){total[i] = bArr[i];}bArr = new Byte[100];}Is that really more efficient - or have i written it badly! Please give an example!
RenegadeAndy
no no no no, I mean simply { byte total[] = new [instrm.available()]; instrm.read(total,0,total.length); }and if you then needed it as a String, use { String asString = String(total,0,total.length,"utf-8"); // assume utf8 :-) }
steelbytes
A: 

What about this. Seems to give better performance.

byte[] bytes = new byte[1000];

StringBuilder x = new StringBuilder();

int numRead = 0;
while ((numRead = is.read(bytes)) >= 0) {
    x.append(new String(bytes, 0, numRead));
}

Edit: Actually this sort of encompasses both steelbytes and Maurice Perry's

Adrian
The problem is - I dont know the size of the thing im reading before i start - so might need some form of array growing as well. Inless you can query an InputStream or URL through http to find out how big the thing im retrieving is to optimise the size of the byte array. I have to be efficient as its on a mobile device which is the main problem!However thanks for that idea - Will give it a shot tonight and let you know how it handles in terms of performance gain!
RenegadeAndy
I don't think the size of the incoming stream is that important. The above code reads 1000 bytes at a time but you could increase/decrease that size. With my testing it didn't make that much difference weather I used 1000/10000 bytes. That was just a simple Java app though. It may be more important on a mobile device.
Adrian
+3  A: 

The problem in your code is that it's creating lots of heavy String objects, copying all its contents and doing operations with them. To drastically improve it you should use StringBuilder, that avoid to instantiate new String objects on each append, it directly uses the internal char arrays without copy them. The implementation for your case would be something like that:

BufferedReader r = new BufferedReader(new InputStreamReader(inputStream));
StringBuilder total = new StringBuilder();
String line;
while ((line = r.readLine()) != null) {
    total.append(line);
}

After that you can use total as a CharSequence for lots of cases without convert it to String. If you need to do it, use total.toString() after the loop.

I'll try to explain it better...

  • a += b (or a = a + b), being a and b Strings, copies a and b chars to a new object (note that you are also copying a, that contains the not-small accumulated String), and you are doing those copies on each iteration. Copying some Kbs and creating some objects lots of times is expensive.
  • a.append(b), being a a StringBuilder, directly appends b contents to a, so you don't copy the accumulated String on each iteration.
Jaime Soriano
A: 

Have you tried the built in method to convert a stream to a string? It's part of the Apache Commons library (org.apache.commons.io.IOUtils).

Then your code would be this one line:

String total = IOUtils.toString(inputStream);
Makotosan
I realize this is a late response, but just now happened to stumble across this via a Google search.
Makotosan