harmony-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Tim Ellison <t.p.elli...@gmail.com>
Subject [classlib][archive] Several archive bugfixes and optimizations (HARMONY-6346)
Date Wed, 07 Oct 2009 20:26:42 GMT
Thanks for the patch Jesse.  A specific comment below.

On 03/Oct/2009 01:32, Jesse Wilson (JIRA) wrote:
> Several archive bugfixes and optimizations 
> ------------------------------------------
> The attached patch includes several miscellaneous bugfixes,
> optimizations and simplifications that we created for Android's copy
> of the archive module.  Here's an overview of what's changed:
> 
> InflaterInputStream Removes a bogus magic number check. Previously I
> submitted a patch to get this check to work; but the whole premise is
> flawed. To demonstrate, attempt to inflate data that was deflated
> using non-default settings.
> 
> InflaterInputStream, ZipInputStream, GZIPInputStream These now make
> sure that available() returns 0 when the end of the stream is
> reached. Previously available() could return 1 even if read() was
> guaranteed to return -1.
> 
> GZIPOutputStream Slight performance fix: cast from long to int only
> once
> 
> JarFile Verifies the entry when the last byte is returned. This is
> similar to the available() problem in ZipInputStream etc. Move
> getMetaEntriesImpl() from native to Java.
> 
> ZipFile Limit the amount of memory used while reading files.
> Previously we would create arbitrarily large buffers. Move several
> methods from native to Java.
> 
> MsgHelp Keep resource bundles in memory with soft references Use the
> system classloader always.
> 
> Tests New tests to demonstrate problems above, recovery on broken
> manifests, verification of empty entries

How about we change this new method in JarFile

private byte[] getAllBytesFromStreamAndClose(InputStream is) throws
IOException {
    ByteArrayOutputStream bs = new ByteArrayOutputStream();
    try {
        byte[] buf = new byte[1024];
        while (is.available() > 0) {
            int iRead = is.read(buf, 0, buf.length);
            if (iRead > 0)
                bs.write(buf, 0, iRead);
        }
    } finally {
        is.close();
    }
    return bs.toByteArray();
}

I know that the InputStream here can only be a RAFStream or
InflaterInputStream, and both of them return available() as 1 so long as
the stream is not at the end, but in general I guess it is better to
read to the -1 eof marker rather than check available().

Then it uses a ByteArrayOutputStream every time, which by default has a
32-byte backing array, that grows aggressively, so putting anything near
1K bytes in will result in a ~2K backing array.

So how about we fast track the case (esp. RAFStream) where we get all
the bytes in one read...?

  // Initial read
  byte[] buffer = new byte[1024];
  int count = is.read(buffer);
  int nextByte = is.read();

  // Did we get it all in one read?
  if (nextByte == -1) {
    byte[] dest = new byte[count];
    System.arraycopy(buffer, 0, dest, 0, count);
    return dest;
  }

  // Requires additional reads
  ByteArrayOutputStream baos = new ByteArrayOutputStream(count * 2);
  baos.write(buffer, 0, count);
  baos.write(nextByte);
  while (true) {
    count = is.read(buffer);
    if (count == -1) {
        return baos.toByteArray();
    }
    baos.write(buffer, 0, count);
  }



The same holds true for Manifest#readFully(InputStream).  Even more so
since if we get an eof we don't have to scan for a line ending.

WDYT?

Regards,
Tim


Mime
View raw message