From 17f06a9f0f728f8346500bbea9f7fea33c3334fb Mon Sep 17 00:00:00 2001 From: Jaikiran Pai Date: Tue, 12 Dec 2017 16:52:26 +0530 Subject: [PATCH] BZ-19516 Use BufferedInputStream for reduced memory usage in Zip task, in certain cases --- WHATSNEW | 5 ++ .../org/apache/tools/ant/taskdefs/Zip.java | 56 ++++++++----------- 2 files changed, 27 insertions(+), 34 deletions(-) diff --git a/WHATSNEW b/WHATSNEW index 100b8bfa6..4f928a411 100644 --- a/WHATSNEW +++ b/WHATSNEW @@ -32,6 +32,11 @@ Fixed bugs: up creating a new symlink under the target directory. Bugzilla Report 58683 + * Improvement to the Zip task for reduced memory usage in certain + cases. Thanks to Glen Lewis for reporting the issue and + suggesting the fix. + Bugzilla Report 19516 + Other changes: -------------- diff --git a/src/main/org/apache/tools/ant/taskdefs/Zip.java b/src/main/org/apache/tools/ant/taskdefs/Zip.java index b4bb1dcfb..4fb7365db 100644 --- a/src/main/org/apache/tools/ant/taskdefs/Zip.java +++ b/src/main/org/apache/tools/ant/taskdefs/Zip.java @@ -17,8 +17,7 @@ */ package org.apache.tools.ant.taskdefs; -import java.io.ByteArrayInputStream; -import java.io.ByteArrayOutputStream; +import java.io.BufferedInputStream; import java.io.File; import java.io.IOException; import java.io.InputStream; @@ -1077,7 +1076,7 @@ public class Zip extends MatchingTask { if (keepCompression) { doCompress = (ze.getMethod() == ZipEntry.DEFLATED); } - try (InputStream is = zf.getInputStream(ze)) { + try (final BufferedInputStream is = new BufferedInputStream(zf.getInputStream(ze))) { zipFile(is, zOut, prefix + name, ze.getTime(), fromArchive, mode, ze.getExtraFields(true)); } finally { @@ -1085,7 +1084,7 @@ public class Zip extends MatchingTask { } } } else { - try (InputStream is = r.getInputStream()) { + try (final BufferedInputStream is = new BufferedInputStream(r.getInputStream())) { zipFile(is, zOut, prefix + name, r.getLastModified(), fromArchive, mode, r instanceof ZipResource ? ((ZipResource) r).getExtraFields() : null); @@ -1776,7 +1775,7 @@ public class Zip extends MatchingTask { * @since Ant 1.5.2 * @throws IOException on error */ - protected void zipFile(InputStream in, final ZipOutputStream zOut, final String vPath, + protected void zipFile(final InputStream in, final ZipOutputStream zOut, final String vPath, final long lastModified, final File fromArchive, final int mode) throws IOException { // fromArchive is used in subclasses overriding this method @@ -1806,7 +1805,12 @@ public class Zip extends MatchingTask { final ZipEntry ze = new ZipEntry(vPath); ze.setTime(fixedModTime != null ? modTimeMillis : lastModified); ze.setMethod(doCompress ? ZipEntry.DEFLATED : ZipEntry.STORED); - + // if the input stream doesn't support mark/reset ability, we wrap it in a + // stream that adds that support. + // Note: We do *not* close this newly created wrapping input stream, since + // we don't "own" the underlying input stream that's passed to us and closing + // that is the responsibility of the caller. + final InputStream markableInputStream = in.markSupported() ? in : new BufferedInputStream(in); /* * ZipOutputStream.putNextEntry expects the ZipEntry to * know its size and the CRC sum before you start writing @@ -1817,31 +1821,15 @@ public class Zip extends MatchingTask { if (!zOut.isSeekable() && !doCompress) { long size = 0; final CRC32 cal = new CRC32(); - if (!in.markSupported()) { - // Store data into a byte[] - final ByteArrayOutputStream bos = new ByteArrayOutputStream(); - - final byte[] buffer = new byte[BUFFER_SIZE]; - int count = 0; - do { - size += count; - cal.update(buffer, 0, count); - bos.write(buffer, 0, count); - count = in.read(buffer, 0, buffer.length); - } while (count != -1); - in = new ByteArrayInputStream(bos.toByteArray()); - - } else { - in.mark(Integer.MAX_VALUE); - final byte[] buffer = new byte[BUFFER_SIZE]; - int count = 0; - do { - size += count; - cal.update(buffer, 0, count); - count = in.read(buffer, 0, buffer.length); - } while (count != -1); - in.reset(); - } + markableInputStream.mark(Integer.MAX_VALUE); + final byte[] buffer = new byte[BUFFER_SIZE]; + int count = 0; + do { + size += count; + cal.update(buffer, 0, count); + count = markableInputStream.read(buffer, 0, buffer.length); + } while (count != -1); + markableInputStream.reset(); ze.setSize(size); ze.setCrc(cal.getValue()); } @@ -1860,7 +1848,7 @@ public class Zip extends MatchingTask { if (count != 0) { zOut.write(buffer, 0, count); } - count = in.read(buffer, 0, buffer.length); + count = markableInputStream.read(buffer, 0, buffer.length); } while (count != -1); } addedFiles.add(vPath); @@ -1916,9 +1904,9 @@ public class Zip extends MatchingTask { getLocation()); } - try (InputStream fIn = Files.newInputStream(file.toPath())) { + try (final BufferedInputStream bIn = new BufferedInputStream(Files.newInputStream(file.toPath()))) { // ZIPs store time with a granularity of 2 seconds, round up - zipFile(fIn, zOut, vPath, + zipFile(bIn, zOut, vPath, file.lastModified() + (roundUp ? ROUNDUP_MILLIS : 0), null, mode); }