You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@commons.apache.org by kr...@apache.org on 2015/01/09 20:13:16 UTC
svn commit: r1650632 - in
/commons/proper/compress/trunk/src/main/java/org/apache/commons/compress/archivers/zip:
ZipArchiveEntry.java ZipArchiveOutputStream.java
Author: krosenvold
Date: Fri Jan 9 19:13:15 2015
New Revision: 1650632
URL: http://svn.apache.org/r1650632
Log:
Changed ZipArchiveEntry to use more optimized data structure for
an overall performance improvement of about 10% for the use case
"many small files", for instance a jar file.
LinkedHashMap was not a very good structure for such small lists
and performs badly in terms of locality
Modified:
commons/proper/compress/trunk/src/main/java/org/apache/commons/compress/archivers/zip/ZipArchiveEntry.java
commons/proper/compress/trunk/src/main/java/org/apache/commons/compress/archivers/zip/ZipArchiveOutputStream.java
Modified: commons/proper/compress/trunk/src/main/java/org/apache/commons/compress/archivers/zip/ZipArchiveEntry.java
URL: http://svn.apache.org/viewvc/commons/proper/compress/trunk/src/main/java/org/apache/commons/compress/archivers/zip/ZipArchiveEntry.java?rev=1650632&r1=1650631&r2=1650632&view=diff
==============================================================================
--- commons/proper/compress/trunk/src/main/java/org/apache/commons/compress/archivers/zip/ZipArchiveEntry.java (original)
+++ commons/proper/compress/trunk/src/main/java/org/apache/commons/compress/archivers/zip/ZipArchiveEntry.java Fri Jan 9 19:13:15 2015
@@ -23,7 +23,6 @@ import java.io.File;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Date;
-import java.util.LinkedHashMap;
import java.util.List;
import java.util.zip.ZipException;
@@ -81,11 +80,12 @@ public class ZipArchiveEntry extends jav
private int internalAttributes = 0;
private int platform = PLATFORM_FAT;
private long externalAttributes = 0;
- private LinkedHashMap<ZipShort, ZipExtraField> extraFields = null;
+ private ZipExtraField[] extraFields;
private UnparseableExtraFieldData unparseableExtra = null;
private String name = null;
private byte[] rawName = null;
private GeneralPurposeBit gpb = new GeneralPurposeBit();
+ private static final ZipExtraField[] noExtraFields = new ZipExtraField[0];
/**
* Creates a new zip entry with the specified name.
@@ -138,7 +138,7 @@ public class ZipArchiveEntry extends jav
this((java.util.zip.ZipEntry) entry);
setInternalAttributes(entry.getInternalAttributes());
setExternalAttributes(entry.getExternalAttributes());
- setExtraFields(entry.getExtraFields(true));
+ setExtraFields(getAllExtraFieldsNoCopy());
setPlatform(entry.getPlatform());
GeneralPurposeBit other = entry.getGeneralPurposeBit();
setGeneralPurposeBit(other == null ? null :
@@ -180,7 +180,7 @@ public class ZipArchiveEntry extends jav
e.setInternalAttributes(getInternalAttributes());
e.setExternalAttributes(getExternalAttributes());
- e.setExtraFields(getExtraFields(true));
+ e.setExtraFields(getAllExtraFieldsNoCopy());
return e;
}
@@ -307,14 +307,15 @@ public class ZipArchiveEntry extends jav
* @param fields an array of extra fields
*/
public void setExtraFields(ZipExtraField[] fields) {
- extraFields = new LinkedHashMap<ZipShort, ZipExtraField>();
+ List<ZipExtraField> newFields = new ArrayList<ZipExtraField>();
for (ZipExtraField field : fields) {
if (field instanceof UnparseableExtraFieldData) {
unparseableExtra = (UnparseableExtraFieldData) field;
} else {
- extraFields.put(field.getHeaderId(), field);
+ newFields.add( field);
}
}
+ extraFields = newFields.toArray(new ZipExtraField[newFields.size()]);
setExtra();
}
@@ -323,7 +324,8 @@ public class ZipArchiveEntry extends jav
* @return an array of the extra fields
*/
public ZipExtraField[] getExtraFields() {
- return getExtraFields(false);
+ final ZipExtraField[] parseableExtraFields = getParseableExtraFields();
+ return Arrays.copyOf(parseableExtraFields, parseableExtraFields.length);
}
/**
@@ -336,20 +338,48 @@ public class ZipArchiveEntry extends jav
* @since 1.1
*/
public ZipExtraField[] getExtraFields(boolean includeUnparseable) {
+ return includeUnparseable ?
+ getAllExtraFields() :
+ getParseableExtraFields();
+ }
+
+ private ZipExtraField[] getParseableExtraFields() {
if (extraFields == null) {
- return !includeUnparseable || unparseableExtra == null
- ? new ZipExtraField[0]
- : new ZipExtraField[] { unparseableExtra };
- }
- List<ZipExtraField> result =
- new ArrayList<ZipExtraField>(extraFields.values());
- if (includeUnparseable && unparseableExtra != null) {
- result.add(unparseableExtra);
+ return noExtraFields;
}
- return result.toArray(new ZipExtraField[result.size()]);
+ return extraFields;
}
/**
+ * Get all extra fields, including unparseable ones.
+ * @return An array of all extra fields. Not necessarily a copy of internal data structures, hence private method
+ */
+ private ZipExtraField[] getAllExtraFieldsNoCopy() {
+ if (extraFields == null) {
+ return getUnparseableOnly();
+ }
+ return unparseableExtra != null ? getMergedFields() : extraFields;
+ }
+
+ private ZipExtraField[] copyOf(ZipExtraField[] src){
+ return Arrays.copyOf(src, src.length);
+ }
+
+ private ZipExtraField[] getMergedFields() {
+ final ZipExtraField[] zipExtraFields = Arrays.copyOf(extraFields, extraFields.length + 1);
+ zipExtraFields[zipExtraFields.length] = unparseableExtra;
+ return zipExtraFields;
+ }
+
+ private ZipExtraField[] getUnparseableOnly() {
+ return unparseableExtra == null ? noExtraFields : new ZipExtraField[] { unparseableExtra };
+ }
+
+ private ZipExtraField[] getAllExtraFields() {
+ final ZipExtraField[] allExtraFieldsNoCopy = getAllExtraFieldsNoCopy();
+ return (allExtraFieldsNoCopy == extraFields) ? copyOf( allExtraFieldsNoCopy) : allExtraFieldsNoCopy;
+ }
+ /**
* Adds an extra field - replacing an already present extra field
* of the same type.
*
@@ -362,9 +392,15 @@ public class ZipArchiveEntry extends jav
unparseableExtra = (UnparseableExtraFieldData) ze;
} else {
if (extraFields == null) {
- extraFields = new LinkedHashMap<ZipShort, ZipExtraField>();
+ extraFields = new ZipExtraField[]{ ze};
+ } else {
+ if (getExtraField(ze.getHeaderId())!= null){
+ removeExtraField(ze.getHeaderId());
+ }
+ final ZipExtraField[] zipExtraFields = Arrays.copyOf(extraFields, extraFields.length + 1);
+ zipExtraFields[zipExtraFields.length -1] = ze;
+ extraFields = zipExtraFields;
}
- extraFields.put(ze.getHeaderId(), ze);
}
setExtra();
}
@@ -380,12 +416,15 @@ public class ZipArchiveEntry extends jav
if (ze instanceof UnparseableExtraFieldData) {
unparseableExtra = (UnparseableExtraFieldData) ze;
} else {
- LinkedHashMap<ZipShort, ZipExtraField> copy = extraFields;
- extraFields = new LinkedHashMap<ZipShort, ZipExtraField>();
- extraFields.put(ze.getHeaderId(), ze);
- if (copy != null) {
- copy.remove(ze.getHeaderId());
- extraFields.putAll(copy);
+ if (getExtraField(ze.getHeaderId()) != null){
+ removeExtraField(ze.getHeaderId());
+ }
+ ZipExtraField[] copy = extraFields;
+ int newLen = extraFields != null ? extraFields.length + 1: 1;
+ extraFields = new ZipExtraField[newLen];
+ extraFields[0] = ze;
+ if (copy != null){
+ System.arraycopy(copy, 0, extraFields, 1, extraFields.length - 1);
}
}
setExtra();
@@ -399,9 +438,17 @@ public class ZipArchiveEntry extends jav
if (extraFields == null) {
throw new java.util.NoSuchElementException();
}
- if (extraFields.remove(type) == null) {
+
+ List<ZipExtraField> newResult = new ArrayList<ZipExtraField>();
+ for (ZipExtraField extraField : extraFields) {
+ if (!type.equals(extraField.getHeaderId())){
+ newResult.add( extraField);
+ }
+ }
+ if (extraFields.length == newResult.size()) {
throw new java.util.NoSuchElementException();
}
+ extraFields = newResult.toArray(new ZipExtraField[newResult.size()]);
setExtra();
}
@@ -425,7 +472,11 @@ public class ZipArchiveEntry extends jav
*/
public ZipExtraField getExtraField(ZipShort type) {
if (extraFields != null) {
- return extraFields.get(type);
+ for (ZipExtraField extraField : extraFields) {
+ if (type.equals(extraField.getHeaderId())) {
+ return extraField;
+ }
+ }
}
return null;
}
@@ -470,7 +521,7 @@ public class ZipArchiveEntry extends jav
* modify super's data directly.
*/
protected void setExtra() {
- super.setExtra(ExtraFieldUtils.mergeLocalFileDataData(getExtraFields(true)));
+ super.setExtra(ExtraFieldUtils.mergeLocalFileDataData(getAllExtraFieldsNoCopy()));
}
/**
@@ -501,7 +552,7 @@ public class ZipArchiveEntry extends jav
* @return the central directory extra data
*/
public byte[] getCentralDirectoryExtra() {
- return ExtraFieldUtils.mergeCentralDirectoryData(getExtraFields(true));
+ return ExtraFieldUtils.mergeCentralDirectoryData(getAllExtraFieldsNoCopy());
}
/**
@@ -528,7 +579,7 @@ public class ZipArchiveEntry extends jav
*/
protected void setName(String name) {
if (name != null && getPlatform() == PLATFORM_FAT
- && name.indexOf("/") == -1) {
+ && !name.contains("/")) {
name = name.replace('\\', '/');
}
this.name = name;
Modified: commons/proper/compress/trunk/src/main/java/org/apache/commons/compress/archivers/zip/ZipArchiveOutputStream.java
URL: http://svn.apache.org/viewvc/commons/proper/compress/trunk/src/main/java/org/apache/commons/compress/archivers/zip/ZipArchiveOutputStream.java?rev=1650632&r1=1650631&r2=1650632&view=diff
==============================================================================
--- commons/proper/compress/trunk/src/main/java/org/apache/commons/compress/archivers/zip/ZipArchiveOutputStream.java (original)
+++ commons/proper/compress/trunk/src/main/java/org/apache/commons/compress/archivers/zip/ZipArchiveOutputStream.java Fri Jan 9 19:13:15 2015
@@ -1023,10 +1023,7 @@ public class ZipArchiveOutputStream exte
putShort(versionNeededToExtract(zipMethod, hasZip64Extra(ze)), buf, LFH_VERSION_NEEDED_OFFSET);
}
- GeneralPurposeBit generalPurposeBit = getGeneralPurposeBits(zipMethod,
- !encodable
- && fallbackToUTF8
- );
+ GeneralPurposeBit generalPurposeBit = getGeneralPurposeBits(zipMethod, !encodable && fallbackToUTF8);
generalPurposeBit.encode(buf, LFH_GPB_OFFSET);
// compression method
Re: svn commit: r1650632 - in /commons/proper/compress/trunk/src/main/java/org/apache/commons/compress/archivers/zip:
ZipArchiveEntry.java ZipArchiveOutputStream.java
Posted by Kristian Rosenvold <kr...@gmail.com>.
I see you fixed this. Thanks.
Kristian
2015-01-11 19:58 GMT+01:00 Stefan Bodewig <bo...@apache.org>:
> On 2015-01-09, <kr...@apache.org> wrote:
>
>> @@ -336,20 +338,48 @@ public class ZipArchiveEntry extends jav
>> * @since 1.1
>> */
>> public ZipExtraField[] getExtraFields(boolean includeUnparseable) {
>>+ return includeUnparseable ?
>>+ getAllExtraFields() :
>>+ getParseableExtraFields();
>>+ }
>
> This is inconsistent WRT copying arrays or returning the internal
> structure. getAllExtraFields will create a copy of extraFields (unless
> there has been unparseable data), but getParseableExtraFields() does
> not.
>
> I think it would be good to always create copies here, maybe call the
> no-arg version of getExtraFields instead of getParseableExtraFields?
>
> Stefan
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: svn commit: r1650632 - in /commons/proper/compress/trunk/src/main/java/org/apache/commons/compress/archivers/zip: ZipArchiveEntry.java ZipArchiveOutputStream.java
Posted by Stefan Bodewig <bo...@apache.org>.
On 2015-01-09, <kr...@apache.org> wrote:
> @@ -336,20 +338,48 @@ public class ZipArchiveEntry extends jav
> * @since 1.1
> */
> public ZipExtraField[] getExtraFields(boolean includeUnparseable) {
>+ return includeUnparseable ?
>+ getAllExtraFields() :
>+ getParseableExtraFields();
>+ }
This is inconsistent WRT copying arrays or returning the internal
structure. getAllExtraFields will create a copy of extraFields (unless
there has been unparseable data), but getParseableExtraFields() does
not.
I think it would be good to always create copies here, maybe call the
no-arg version of getExtraFields instead of getParseableExtraFields?
Stefan
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org