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


Reply via email to