Add Image Optimisations

build.gradle

@@ -199,7 +199,7 @@ project('ooxml') {
 		compile 'org.apache.santuario:xmlsec:2.0.6'
 		compile 'org.bouncycastle:bcpkix-jdk15on:1.54'
 		compile 'com.github.virtuald:curvesapi:1.04'
-		
+
 		// for ooxml-lite, should we move this somewhere else?
 		compile 'junit:junit:4.12'
 
@@ -210,6 +210,8 @@ project('ooxml') {
 
 		testCompile 'junit:junit:4.12'
 		testCompile project(path: ':main', configuration: 'tests')
+		testCompile 'org.openjdk.jmh:jmh-core:1.15'
+		testCompile 'org.openjdk.jmh:jmh-generator-annprocess:1.15'
 	}
 
 	// TOOD: we should not duplicate this task in each project, but I did not figure out how to inject the artifactId for each project

build.xml

@@ -163,6 +163,11 @@ under the License.
     <property name="main.log4j.url" value="${repository.m2}/maven2/log4j/log4j/1.2.17/log4j-1.2.17.jar"/>
     <property name="main.junit.jar" location="${main.lib}/junit-4.12.jar"/>
     <property name="main.junit.url" value="${repository.m2}/maven2/junit/junit/4.12/junit-4.12.jar"/>
+    <property name="main.jmh.jar" location="${main.lib}/jmh-core-1.15.jar"/>
+    <property name="main.jmh.url" value="${repository.m2}/maven2/org/openjdk/jmh/jmh-core/1.15/jmh-core-1.15.jar"/>
+    <property name="main.jmhAnnotation.jar" location="${main.lib}/jmh-generator-annprocess-1.15.jar"/>
+    <property name="main.jmhAnnotation.url" value="${repository.m2}/maven2/org/openjdk/jmh/jmh-generator-annprocess/1.15/jmh-generator-annprocess-1.15.jar"/>
+
     <property name="main.hamcrest.jar" location="${main.lib}/hamcrest-core-1.3.jar"/>
     <property name="main.hamcrest.url" value="${repository.m2}/maven2/org/hamcrest/hamcrest-core/1.3/hamcrest-core-1.3.jar"/>
     <property name="main.ant.jar" location="${main.lib}/ant-1.9.4.jar"/>
@@ -321,6 +326,8 @@ under the License.
         <pathelement location="${main.commons-codec.jar}"/>
         <pathelement location="${main.log4j.jar}"/>
         <pathelement location="${main.junit.jar}"/>
+        <pathelement location="${main.jmh.jar}"/>
+        <pathelement location="${main.jmhAnnotation.jar}"/>
         <pathelement location="${main.hamcrest.jar}"/>
         <pathelement location="${main.commons-collections4.jar}"/>
     </path>
@@ -599,6 +606,8 @@ under the License.
                     <available file="${main.commons-codec.jar}"/>
                     <available file="${main.log4j.jar}"/>
                     <available file="${main.junit.jar}"/>
+                    <available file="${main.jmh.jar}"/>
+                    <available file="${main.jmhAnnotation.jar}"/>
                     <available file="${main.hamcrest.jar}"/>
                     <available file="${main.ant.jar}"/>
                     <available file="${main.antlauncher.jar}"/>
@@ -624,6 +633,8 @@ under the License.
         <downloadfile src="${main.commons-codec.url}" dest="${main.commons-codec.jar}"/>
         <downloadfile src="${main.log4j.url}" dest="${main.log4j.jar}"/>
         <downloadfile src="${main.junit.url}" dest="${main.junit.jar}"/>
+        <downloadfile src="${main.jmh.url}" dest="${main.jmh.jar}"/>
+        <downloadfile src="${main.jmhAnnotation.url}" dest="${main.jmhAnnotation.jar}"/>
         <downloadfile src="${main.hamcrest.url}" dest="${main.hamcrest.jar}"/>
         <downloadfile src="${main.ant.url}" dest="${main.ant.jar}"/>
         <downloadfile src="${main.antlauncher.url}" dest="${main.antlauncher.jar}"/>
@@ -2264,6 +2275,8 @@ under the License.
             <auxClasspath path="${main.commons-codec.jar}" />
             <auxClasspath path="${main.commons-logging.jar}" />
             <auxClasspath path="${main.junit.jar}" />
+            <auxClasspath path="${main.jmh.jar}"/>
+            <auxClasspath path="${main.jmhAnnotation.jar}"/>
             <auxClasspath path="${main.ant.jar}" />
             <sourcePath path="src/java" />
             <sourcePath path="src/ooxml/java" />

src/ooxml/java/org/apache/poi/POIXMLDocumentPart.java

@@ -268,7 +268,7 @@ public final String getRelationId(POIXMLDocumentPart part) {
      * @since 3.14-Beta1
      */
     public final RelationPart addRelation(String relId, POIXMLRelation relationshipType, POIXMLDocumentPart part) {
-        PackageRelationship pr = findExistingRelation(part);
+        PackageRelationship pr = this.packagePart.findExistingRelation(part.getPackagePart());
         if (pr == null) {
             PackagePartName ppn = part.getPackagePart().getPartName();
             String relType = relationshipType.getRelation();
@@ -290,30 +290,6 @@ private void addRelation(PackageRelationship pr, POIXMLDocumentPart part) {
 
     }
 
-    /**
-     * Check if the new part was already added before via PackagePart.addRelationship()
-     *
-     * @param part to find the relationship for
-     * @return The existing relationship, or null if there isn't yet one
-     */
-    private PackageRelationship findExistingRelation(POIXMLDocumentPart part) {
-        String ppn = part.getPackagePart().getPartName().getName();
-        try {
-            for (PackageRelationship pr : packagePart.getRelationships()) {
-                if (pr.getTargetMode() == TargetMode.EXTERNAL) {
-                    continue;
-                }
-                PackagePart pp = packagePart.getRelatedPart(pr);
-                if (ppn.equals(pp.getPartName().getName())) {
-                    return pr;
-                }
-            }
-        } catch (InvalidFormatException e) {
-            throw new POIXMLException("invalid package relationships", e);
-        }
-        return null;
-    }
-
     /**
      * Remove the relation to the specified part in this package and remove the
      * part, if it is no longer needed.

src/ooxml/java/org/apache/poi/openxml4j/opc/OPCPackage.java

@@ -649,12 +649,11 @@ public PackagePart getPart(PackagePartName partName) {
 	 */
 	public ArrayList<PackagePart> getPartsByContentType(String contentType) {
 		ArrayList<PackagePart> retArr = new ArrayList<PackagePart>();
-		for (PackagePart part : partList.values()) {
+		for (PackagePart part : partList.sortedValues()) {
 			if (part.getContentType().equals(contentType)) {
 				retArr.add(part);
 			}
 		}
-		Collections.sort(retArr);
 		return retArr;
 	}
 
@@ -697,13 +696,12 @@ public List<PackagePart> getPartsByName(final Pattern namePattern) {
 	    }
 	    Matcher matcher = namePattern.matcher("");
 	    ArrayList<PackagePart> result = new ArrayList<PackagePart>();
-	    for (PackagePart part : partList.values()) {
+	    for (PackagePart part : partList.sortedValues()) {
 	        PackagePartName partName = part.getPartName();
 	        if (matcher.reset(partName.getName()).matches()) {
 	            result.add(part);
 	        }
 	    }
-	    Collections.sort(result);
 	    return result;
 	}
 
@@ -813,9 +811,7 @@ public ArrayList<PackagePart> getParts() throws InvalidFormatException {
 				}
 			}
 		}
-		ArrayList<PackagePart> result = new ArrayList<PackagePart>(partList.values());
-		java.util.Collections.sort(result);
-		return result;
+		return new ArrayList<PackagePart>(partList.sortedValues());
 	}
 
 	/**

src/ooxml/java/org/apache/poi/openxml4j/opc/PackagePart.java

@@ -22,6 +22,7 @@ Licensed to the Apache Software Foundation (ASF) under one or more
 import java.io.OutputStream;
 import java.net.URI;
 import java.net.URISyntaxException;
+import java.util.HashMap;
 
 import org.apache.poi.openxml4j.exceptions.InvalidFormatException;
 import org.apache.poi.openxml4j.exceptions.InvalidOperationException;
@@ -63,6 +64,7 @@ public abstract class PackagePart implements RelationshipSource, Comparable<Pack
 	 */
 	private PackageRelationshipCollection _relationships;
 
+
 	/**
 	 * Constructor.
 	 *
@@ -125,6 +127,16 @@ public PackagePart(OPCPackage pack, PackagePartName partName,
 		this(pack, partName, new ContentType(contentType));
 	}
 
+	/**
+	 * Check if the new part was already added before via PackagePart.addRelationship()
+	 *
+	 * @param packagePart to find the relationship for
+	 * @return The existing relationship, or null if there isn't yet one
+	 */
+	public PackageRelationship findExistingRelation(PackagePart packagePart) {
+		return _relationships.findExistingInternalRelation(packagePart);
+    }
+
 	/**
 	 * Adds an external relationship to a part (except relationships part).
 	 *
@@ -446,11 +458,7 @@ public boolean hasRelationships() {
 	 * @see org.apache.poi.openxml4j.opc.RelationshipSource#isRelationshipExists(org.apache.poi.openxml4j.opc.PackageRelationship)
 	 */
 	public boolean isRelationshipExists(PackageRelationship rel) {
-		for (PackageRelationship r : _relationships) {
-			if (r == rel)
-				return true;
-		}
-        return false;
+		return _relationships.getRelationshipByID(rel.getId()) != null;
 	}
 
    /**
@@ -464,27 +472,26 @@ public PackagePart getRelatedPart(PackageRelationship rel) throws InvalidFormatE
        if(! isRelationshipExists(rel)) {
           throw new IllegalArgumentException("Relationship " + rel + " doesn't start with this part " + _partName);
        }
-
-       // Get the target URI, excluding any relative fragments
-       URI target = rel.getTargetURI();
-       if(target.getFragment() != null) {
-          String t = target.toString();
-          try {
-             target = new URI( t.substring(0, t.indexOf('#')) );
-          } catch(URISyntaxException e) {
-             throw new InvalidFormatException("Invalid target URI: " + target);
-          }
-       }
-
-       // Turn that into a name, and fetch
-       PackagePartName relName = PackagingURIHelper.createPartName(target);
-       PackagePart part = _container.getPart(relName);
-       if (part == null) {
-           throw new IllegalArgumentException("No part found for relationship " + rel);
-       }
-       return part;
-   }
-
+	   // Get the target URI, excluding any relative fragments
+	   URI target = rel.getTargetURI();
+	   if(target.getFragment() != null) {
+		   String t = target.toString();
+		   try {
+			   target = new URI( t.substring(0, t.indexOf('#')) );
+		   } catch(URISyntaxException e) {
+			   throw new InvalidFormatException("Invalid target URI: " + target);
+		   }
+	   }
+
+		// Turn that into a name, and fetch
+		PackagePartName relName = PackagingURIHelper.createPartName(target);
+		PackagePart part = _container.getPart(relName);
+		if (part == null) {
+            throw new IllegalArgumentException("No part found for relationship " + rel);
+        }
+		return part;
+	}
+
 	/**
 	 * Get the input stream of this part to read its content.
 	 *

src/ooxml/java/org/apache/poi/openxml4j/opc/PackagePartCollection.java

@@ -17,8 +17,8 @@ Licensed to the Apache Software Foundation (ASF) under one or more
 
 package org.apache.poi.openxml4j.opc;
 
-import java.util.ArrayList;
-import java.util.TreeMap;
+import java.io.Serializable;
+import java.util.*;
 
 import org.apache.poi.openxml4j.exceptions.InvalidOperationException;
 
@@ -28,21 +28,19 @@ Licensed to the Apache Software Foundation (ASF) under one or more
  * @author Julien Chable
  * @version 0.1
  */
-public final class PackagePartCollection extends
-		TreeMap<PackagePartName, PackagePart> {
+public final class PackagePartCollection implements Serializable {
 
-	private static final long serialVersionUID = 2515031135957635515L;
+	private static final long serialVersionUID = 2515031135957635517L;
 
 	/**
-	 * Arraylist use to store this collection part names as string for rule
+	 * HashSet use to store this collection part names as string for rule
 	 * M1.11 optimized checking.
 	 */
-	private ArrayList<String> registerPartNameStr = new ArrayList<String>();
+	private HashSet<String> registerPartNameStr = new HashSet<String>();
+
+
+	private final HashMap<PackagePartName, PackagePart> packagePartLookup = new HashMap<PackagePartName, PackagePart>();
 
-	@Override
-	public Object clone() {
-		return super.clone();
-	}
 
 	/**
 	 * Check rule [M1.11]: a package implementer shall neither create nor
@@ -53,11 +51,10 @@ public Object clone() {
 	 *                Throws if you try to add a part with a name derived from
 	 *                another part name.
 	 */
-	@Override
 	public PackagePart put(PackagePartName partName, PackagePart part) {
 		String[] segments = partName.getURI().toASCIIString().split(
 				PackagingURIHelper.FORWARD_SLASH_STRING);
-		StringBuffer concatSeg = new StringBuffer();
+		StringBuilder concatSeg = new StringBuilder();
 		for (String seg : segments) {
 			if (!seg.equals(""))
 				concatSeg.append(PackagingURIHelper.FORWARD_SLASH_CHAR);
@@ -68,14 +65,35 @@ public PackagePart put(PackagePartName partName, PackagePart part) {
 			}
 		}
 		this.registerPartNameStr.add(partName.getName());
-		return super.put(partName, part);
+		return packagePartLookup.put(partName, part);
 	}
 
-	@Override
-	public PackagePart remove(Object key) {
-		if (key instanceof PackagePartName) {
-			this.registerPartNameStr.remove(((PackagePartName) key).getName());
-		}
-		return super.remove(key);
+	public PackagePart remove(PackagePartName key) {
+		this.registerPartNameStr.remove(key.getName());
+		return packagePartLookup.remove(key);
+	}
+
+
+	/**
+	 * The values themselves should be returned in sorted order. Doing it here
+	 * avoids paying the high cost of Natural Ordering per insertion.
+	 */
+	public Collection<PackagePart> sortedValues() {
+		ArrayList<PackagePart> packageParts = new ArrayList<PackagePart>(packagePartLookup.values());
+		Collections.sort(packageParts);
+		return packageParts;
+
+	}
+
+	public boolean containsKey(PackagePartName partName) {
+		return packagePartLookup.containsKey(partName);
+	}
+
+	public PackagePart get(PackagePartName partName) {
+		return packagePartLookup.get(partName);
+	}
+
+	public int size() {
+		return packagePartLookup.size();
 	}
 }