You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@harmony.apache.org by "Jesse Wilson (JIRA)" <ji...@apache.org> on 2009/10/03 02:32:23 UTC
[jira] Created: (HARMONY-6346) Several archive bugfixes and
optimizations
Several archive bugfixes and optimizations
------------------------------------------
Key: HARMONY-6346
URL: https://issues.apache.org/jira/browse/HARMONY-6346
Project: Harmony
Issue Type: Improvement
Components: Classlib
Environment: SVN Revision: 820775
Reporter: Jesse Wilson
Priority: Minor
Attachments: archive_from_android.patch
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
Note that there are some problems with the patch, so it should not be applied! I will to upload an improved patch early next week. I figured I'd let everyone preview the patch in its current state; the problems are minor and include removing dead code and fixing tests with invalid assumptions.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
Re: [classlib][archive] Several archive bugfixes and optimizations
(HARMONY-6346)
Posted by Jesse Wilson <je...@google.com>.
On Wed, Oct 7, 2009 at 1:26 PM, Tim Ellison <t....@gmail.com> wrote:
> 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?
>
That sounds fine. Or perhaps just right-size the ByteArrayOutputStream to
something reasonable, like 256 bytes.
[classlib][archive] Several archive bugfixes and optimizations (HARMONY-6346)
Posted by Tim Ellison <t....@gmail.com>.
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
[jira] Updated: (HARMONY-6346) [classlib] [archive] Several archive
bugfixes and optimizations
Posted by "Mark Hindess (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HARMONY-6346?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Mark Hindess updated HARMONY-6346:
----------------------------------
Fix Version/s: 5.0M12
Not really must fix but we must resolve the regressions at least and I want to make sure it shows up in our release checklist.
> [classlib] [archive] Several archive bugfixes and optimizations
> ---------------------------------------------------------------
>
> Key: HARMONY-6346
> URL: https://issues.apache.org/jira/browse/HARMONY-6346
> Project: Harmony
> Issue Type: Improvement
> Components: Classlib
> Environment: SVN Revision: 820775
> Reporter: Jesse Wilson
> Assignee: Tim Ellison
> Fix For: 5.0M12
>
> Attachments: archive_from_android.patch, archive_from_android_2.patch, EmptyEntries_signed.jar
>
> Original Estimate: 48h
> Remaining Estimate: 48h
>
> 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
> Note that there are some problems with the patch, so it should not be applied! I will to upload an improved patch early next week. I figured I'd let everyone preview the patch in its current state; the problems are minor and include removing dead code and fixing tests with invalid assumptions.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (HARMONY-6346) Several archive bugfixes and
optimizations
Posted by "Jesse Wilson (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HARMONY-6346?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Jesse Wilson updated HARMONY-6346:
----------------------------------
Priority: Major (was: Minor)
Promoting to major priority. Streaming unzipped entries, and inflating files encoded with non-default settings are significant fixes to the current codebase.
> Several archive bugfixes and optimizations
> ------------------------------------------
>
> Key: HARMONY-6346
> URL: https://issues.apache.org/jira/browse/HARMONY-6346
> Project: Harmony
> Issue Type: Improvement
> Components: Classlib
> Environment: SVN Revision: 820775
> Reporter: Jesse Wilson
> Assignee: Tim Ellison
> Attachments: archive_from_android.patch, archive_from_android_2.patch
>
> Original Estimate: 48h
> Remaining Estimate: 48h
>
> 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
> Note that there are some problems with the patch, so it should not be applied! I will to upload an improved patch early next week. I figured I'd let everyone preview the patch in its current state; the problems are minor and include removing dead code and fixing tests with invalid assumptions.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Closed: (HARMONY-6346) [classlib] [archive] Several archive
bugfixes and optimizations
Posted by "Mark Hindess (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HARMONY-6346?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Mark Hindess closed HARMONY-6346.
---------------------------------
> [classlib] [archive] Several archive bugfixes and optimizations
> ---------------------------------------------------------------
>
> Key: HARMONY-6346
> URL: https://issues.apache.org/jira/browse/HARMONY-6346
> Project: Harmony
> Issue Type: Improvement
> Components: Classlib
> Environment: SVN Revision: 820775
> Reporter: Jesse Wilson
> Assignee: Tim Ellison
> Fix For: 5.0M12
>
> Attachments: archive_from_android.patch, archive_from_android_2.patch, EmptyEntries_signed.jar
>
> Original Estimate: 48h
> Remaining Estimate: 48h
>
> 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
> Note that there are some problems with the patch, so it should not be applied! I will to upload an improved patch early next week. I figured I'd let everyone preview the patch in its current state; the problems are minor and include removing dead code and fixing tests with invalid assumptions.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Resolved: (HARMONY-6346) [classlib] [archive] Several
archive bugfixes and optimizations
Posted by "Mark Hindess (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HARMONY-6346?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Mark Hindess resolved HARMONY-6346.
-----------------------------------
Resolution: Fixed
Agreed. Thanks Tim. (It was late and I wanted to make sure this problem was captured somewhere before I went to sleep. ;-)
> [classlib] [archive] Several archive bugfixes and optimizations
> ---------------------------------------------------------------
>
> Key: HARMONY-6346
> URL: https://issues.apache.org/jira/browse/HARMONY-6346
> Project: Harmony
> Issue Type: Improvement
> Components: Classlib
> Environment: SVN Revision: 820775
> Reporter: Jesse Wilson
> Assignee: Tim Ellison
> Fix For: 5.0M12
>
> Attachments: archive_from_android.patch, archive_from_android_2.patch, EmptyEntries_signed.jar
>
> Original Estimate: 48h
> Remaining Estimate: 48h
>
> 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
> Note that there are some problems with the patch, so it should not be applied! I will to upload an improved patch early next week. I figured I'd let everyone preview the patch in its current state; the problems are minor and include removing dead code and fixing tests with invalid assumptions.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Assigned: (HARMONY-6346) Several archive bugfixes and
optimizations
Posted by "Tim Ellison (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HARMONY-6346?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Tim Ellison reassigned HARMONY-6346:
------------------------------------
Assignee: Tim Ellison
> Several archive bugfixes and optimizations
> ------------------------------------------
>
> Key: HARMONY-6346
> URL: https://issues.apache.org/jira/browse/HARMONY-6346
> Project: Harmony
> Issue Type: Improvement
> Components: Classlib
> Environment: SVN Revision: 820775
> Reporter: Jesse Wilson
> Assignee: Tim Ellison
> Priority: Minor
> Attachments: archive_from_android.patch
>
> Original Estimate: 48h
> Remaining Estimate: 48h
>
> 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
> Note that there are some problems with the patch, so it should not be applied! I will to upload an improved patch early next week. I figured I'd let everyone preview the patch in its current state; the problems are minor and include removing dead code and fixing tests with invalid assumptions.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (HARMONY-6346) [classlib] [archive] Several archive
bugfixes and optimizations
Posted by "Tim Ellison (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HARMONY-6346?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Tim Ellison updated HARMONY-6346:
---------------------------------
Summary: [classlib] [archive] Several archive bugfixes and optimizations (was: Several archive bugfixes and optimizations)
> [classlib] [archive] Several archive bugfixes and optimizations
> ---------------------------------------------------------------
>
> Key: HARMONY-6346
> URL: https://issues.apache.org/jira/browse/HARMONY-6346
> Project: Harmony
> Issue Type: Improvement
> Components: Classlib
> Environment: SVN Revision: 820775
> Reporter: Jesse Wilson
> Assignee: Tim Ellison
> Attachments: archive_from_android.patch, archive_from_android_2.patch, EmptyEntries_signed.jar
>
> Original Estimate: 48h
> Remaining Estimate: 48h
>
> 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
> Note that there are some problems with the patch, so it should not be applied! I will to upload an improved patch early next week. I figured I'd let everyone preview the patch in its current state; the problems are minor and include removing dead code and fixing tests with invalid assumptions.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (HARMONY-6346) Several archive bugfixes and
optimizations
Posted by "Jesse Wilson (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HARMONY-6346?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Jesse Wilson updated HARMONY-6346:
----------------------------------
Attachment: archive_from_android_2.patch
This completes the patch, addressing a few outstanding test problems and removing the obsolete native methods.
In order to support data deflated with non-default settings, the bogus magic number check must be removed. This prevents us from failing early when inflating the single byte -1, but the code continues to detect a problem.
This patch moves unzip from native code to Java. The primary benefit is that zip entries do not need to be fully-decompressed into an in-memory byte array. This is very important for mobile, and critical anywhere that zip files contain large entries. This regresses our ability to support reset() on zip file entry streams, but this limitation is consistent with the RI.
> Several archive bugfixes and optimizations
> ------------------------------------------
>
> Key: HARMONY-6346
> URL: https://issues.apache.org/jira/browse/HARMONY-6346
> Project: Harmony
> Issue Type: Improvement
> Components: Classlib
> Environment: SVN Revision: 820775
> Reporter: Jesse Wilson
> Assignee: Tim Ellison
> Priority: Minor
> Attachments: archive_from_android.patch, archive_from_android_2.patch
>
> Original Estimate: 48h
> Remaining Estimate: 48h
>
> 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
> Note that there are some problems with the patch, so it should not be applied! I will to upload an improved patch early next week. I figured I'd let everyone preview the patch in its current state; the problems are minor and include removing dead code and fixing tests with invalid assumptions.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (HARMONY-6346) [classlib] [archive] Several
archive bugfixes and optimizations
Posted by "Tim Ellison (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HARMONY-6346?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12763427#action_12763427 ]
Tim Ellison commented on HARMONY-6346:
--------------------------------------
FYI the patch introduced new FindBugs warnings
http://hudson.zones.apache.org/hudson/view/Harmony/job/Harmony-1.5-head-linux-x86_64/466/findbugsResult/new/
> [classlib] [archive] Several archive bugfixes and optimizations
> ---------------------------------------------------------------
>
> Key: HARMONY-6346
> URL: https://issues.apache.org/jira/browse/HARMONY-6346
> Project: Harmony
> Issue Type: Improvement
> Components: Classlib
> Environment: SVN Revision: 820775
> Reporter: Jesse Wilson
> Assignee: Tim Ellison
> Attachments: archive_from_android.patch, archive_from_android_2.patch, EmptyEntries_signed.jar
>
> Original Estimate: 48h
> Remaining Estimate: 48h
>
> 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
> Note that there are some problems with the patch, so it should not be applied! I will to upload an improved patch early next week. I figured I'd let everyone preview the patch in its current state; the problems are minor and include removing dead code and fixing tests with invalid assumptions.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (HARMONY-6346) Several archive bugfixes and
optimizations
Posted by "Jesse Wilson (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HARMONY-6346?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Jesse Wilson updated HARMONY-6346:
----------------------------------
Attachment: archive_from_android.patch
> Several archive bugfixes and optimizations
> ------------------------------------------
>
> Key: HARMONY-6346
> URL: https://issues.apache.org/jira/browse/HARMONY-6346
> Project: Harmony
> Issue Type: Improvement
> Components: Classlib
> Environment: SVN Revision: 820775
> Reporter: Jesse Wilson
> Priority: Minor
> Attachments: archive_from_android.patch
>
> Original Estimate: 48h
> Remaining Estimate: 48h
>
> 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
> Note that there are some problems with the patch, so it should not be applied! I will to upload an improved patch early next week. I figured I'd let everyone preview the patch in its current state; the problems are minor and include removing dead code and fixing tests with invalid assumptions.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (HARMONY-6346) [classlib] [archive] Several
archive bugfixes and optimizations
Posted by "Tim Ellison (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HARMONY-6346?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12782341#action_12782341 ]
Tim Ellison commented on HARMONY-6346:
--------------------------------------
I think this can be re-closed<g>, assuming the regressions Mark refers to are those in HARMONY-6386.
> [classlib] [archive] Several archive bugfixes and optimizations
> ---------------------------------------------------------------
>
> Key: HARMONY-6346
> URL: https://issues.apache.org/jira/browse/HARMONY-6346
> Project: Harmony
> Issue Type: Improvement
> Components: Classlib
> Environment: SVN Revision: 820775
> Reporter: Jesse Wilson
> Assignee: Tim Ellison
> Fix For: 5.0M12
>
> Attachments: archive_from_android.patch, archive_from_android_2.patch, EmptyEntries_signed.jar
>
> Original Estimate: 48h
> Remaining Estimate: 48h
>
> 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
> Note that there are some problems with the patch, so it should not be applied! I will to upload an improved patch early next week. I figured I'd let everyone preview the patch in its current state; the problems are minor and include removing dead code and fixing tests with invalid assumptions.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Reopened: (HARMONY-6346) [classlib] [archive] Several
archive bugfixes and optimizations
Posted by "Mark Hindess (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HARMONY-6346?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Mark Hindess reopened HARMONY-6346:
-----------------------------------
The commit r822846 for this JIRA causes a regression in several tests in the sound module. Specifically:
AudioSystemTest testMixer fails with junit.framework.AssertionFailedError: null at org.apache.harmony.sound.tests.javax.sound.sampled.AudioSystemTest.testMixer (AudioSystemTest.java:61) at java.lang.reflect.AccessibleObject.invokeV(AccessibleObject.java:197)
AudioSystemTest testFormatConversion fails with junit.framework.AssertionFailedError: null at org.apache.harmony.sound.tests.javax.sound.sampled.AudioSystemTest.testFormatConversion (AudioSystemTest.java:128) at java.lang.reflect.AccessibleObject.invokeV(AccessibleObject.java:197)
AudioSystemTest testAudioFile has error javax.sound.sampled.UnsupportedAudioFileException: File is not a supported file type at javax.sound.sampled.AudioSystem.getAudioFileFormat(AudioSystem.java:460) at org.apache.harmony.sound.tests.javax.sound.sampled.AudioSystemTest.testAudioFile (AudioSystemTest.java:43)
AudioSystemTest testAudioInputStream has error javax.sound.sampled.UnsupportedAudioFileException: Could not get audio input stream from input file at javax.sound.sampled.AudioSystem.getAudioInputStream(AudioSystem.java:519) at org.apache.harmony.sound.tests.javax.sound.sampled.AudioSystemTest.testAudioInputStream (AudioSystemTest.java:98)
AudioSystemTest testGetLine has error java.lang.IllegalArgumentException: Could not get line at javax.sound.sampled.AudioSystem.getLine(AudioSystem.java:270) at javax.sound.sampled.AudioSystem.getLine(AudioSystem.java:220) at org.apache.harmony.sound.tests.javax.sound.sampled.AudioSystemTest.testGetLine (AudioSystemTest.java:153)
This is on linux/x86_64 though I think the failures might occur on other platforms too.
> [classlib] [archive] Several archive bugfixes and optimizations
> ---------------------------------------------------------------
>
> Key: HARMONY-6346
> URL: https://issues.apache.org/jira/browse/HARMONY-6346
> Project: Harmony
> Issue Type: Improvement
> Components: Classlib
> Environment: SVN Revision: 820775
> Reporter: Jesse Wilson
> Assignee: Tim Ellison
> Attachments: archive_from_android.patch, archive_from_android_2.patch, EmptyEntries_signed.jar
>
> Original Estimate: 48h
> Remaining Estimate: 48h
>
> 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
> Note that there are some problems with the patch, so it should not be applied! I will to upload an improved patch early next week. I figured I'd let everyone preview the patch in its current state; the problems are minor and include removing dead code and fixing tests with invalid assumptions.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (HARMONY-6346) Several archive bugfixes and
optimizations
Posted by "Jesse Wilson (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HARMONY-6346?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Jesse Wilson updated HARMONY-6346:
----------------------------------
Attachment: EmptyEntries_signed.jar
to working_classlib/support/src/test/java/tests/resources
> Several archive bugfixes and optimizations
> ------------------------------------------
>
> Key: HARMONY-6346
> URL: https://issues.apache.org/jira/browse/HARMONY-6346
> Project: Harmony
> Issue Type: Improvement
> Components: Classlib
> Environment: SVN Revision: 820775
> Reporter: Jesse Wilson
> Assignee: Tim Ellison
> Attachments: archive_from_android.patch, archive_from_android_2.patch, EmptyEntries_signed.jar
>
> Original Estimate: 48h
> Remaining Estimate: 48h
>
> 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
> Note that there are some problems with the patch, so it should not be applied! I will to upload an improved patch early next week. I figured I'd let everyone preview the patch in its current state; the problems are minor and include removing dead code and fixing tests with invalid assumptions.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (HARMONY-6346) [classlib] [archive] Several
archive bugfixes and optimizations
Posted by "Tim Ellison (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HARMONY-6346?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12763190#action_12763190 ]
Tim Ellison commented on HARMONY-6346:
--------------------------------------
Thanks Jesse,
Some interesting changes here. I've applied a subset to start with:
- Having so many whitespace changes in there made it harder to review. While it is good to get formatting/tidy-up changes, best if they are in a separate patch for clarity sake.
- I've not applied the changes to call the MsgHelp class (yet?). I think this is part of a larger discussion that we should have about error messages, but again in any case it feels like a different type of enhancement I can go on to look at now the others are in place.
Please check that you are happy with the patch that was applied at repo revision r822846.
I won't close the JIRA just yet so we won't forget the other improvements.
> [classlib] [archive] Several archive bugfixes and optimizations
> ---------------------------------------------------------------
>
> Key: HARMONY-6346
> URL: https://issues.apache.org/jira/browse/HARMONY-6346
> Project: Harmony
> Issue Type: Improvement
> Components: Classlib
> Environment: SVN Revision: 820775
> Reporter: Jesse Wilson
> Assignee: Tim Ellison
> Attachments: archive_from_android.patch, archive_from_android_2.patch, EmptyEntries_signed.jar
>
> Original Estimate: 48h
> Remaining Estimate: 48h
>
> 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
> Note that there are some problems with the patch, so it should not be applied! I will to upload an improved patch early next week. I figured I'd let everyone preview the patch in its current state; the problems are minor and include removing dead code and fixing tests with invalid assumptions.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Closed: (HARMONY-6346) [classlib] [archive] Several archive
bugfixes and optimizations
Posted by "Jesse Wilson (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HARMONY-6346?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Jesse Wilson closed HARMONY-6346.
---------------------------------
Resolution: Fixed
Looks Good To Me.
I reviewed the commits. The code looks great, even better that the patched I submitted. Thanks Tim.
> [classlib] [archive] Several archive bugfixes and optimizations
> ---------------------------------------------------------------
>
> Key: HARMONY-6346
> URL: https://issues.apache.org/jira/browse/HARMONY-6346
> Project: Harmony
> Issue Type: Improvement
> Components: Classlib
> Environment: SVN Revision: 820775
> Reporter: Jesse Wilson
> Assignee: Tim Ellison
> Attachments: archive_from_android.patch, archive_from_android_2.patch, EmptyEntries_signed.jar
>
> Original Estimate: 48h
> Remaining Estimate: 48h
>
> 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
> Note that there are some problems with the patch, so it should not be applied! I will to upload an improved patch early next week. I figured I'd let everyone preview the patch in its current state; the problems are minor and include removing dead code and fixing tests with invalid assumptions.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.