On Thu, 2006-03-30 at 19:49 -0700, Tom Tromey wrote:
> >>>>> "Tom" == Thomas Fitzsimmons <[EMAIL PROTECTED]> writes:
>
> Tom> + public static final JPEGQTable K1Luminance = new JPEGQTable(new int[]
> Tom> + {
>
> For these it would be mildly better to have a private constructor
> which doesn't copy the argument.
>
> Tom> + int[] tableCopy = new int[table.length];
> Tom> + System.arraycopy(table, 0, tableCopy, 0, table.length);
> Tom> + return tableCopy;
>
> This is: return (int[]) table.clone();
>
> Tom> + public JPEGQTable getScaledInstance(float scaleFactor,
> Tom> + boolean forceBaseline)
> Tom> + {
> Tom> + int[] scaledTable = getTable();
>
> Somehow this should also be able to make a single copy of the table.
> Right now it makes 2 -- one here and one in the constructor.
Thanks for the comments. They're all fixed by the attached patch, which
I've committed.
Tom
2006-03-31 Thomas Fitzsimmons <[EMAIL PROTECTED]>
* javax/imageio/plugins/jpeg/JPEGHuffmanTable.java: Eliminate
unnecessary copying.
* javax/imageio/plugins/jpeg/JPEGQTable.java: Likewise.
Index: javax/imageio/plugins/jpeg/JPEGHuffmanTable.java
===================================================================
RCS file: /sources/classpath/classpath/javax/imageio/plugins/jpeg/JPEGHuffmanTable.java,v
retrieving revision 1.2
diff -u -r1.2 JPEGHuffmanTable.java
--- javax/imageio/plugins/jpeg/JPEGHuffmanTable.java 31 Mar 2006 02:07:31 -0000 1.2
+++ javax/imageio/plugins/jpeg/JPEGHuffmanTable.java 31 Mar 2006 21:50:01 -0000
@@ -55,6 +55,8 @@
*/
private short[] values;
+ // The private constructors are used for these final fields to avoid
+ // unnecessary copying.
/**
* The standard JPEG AC chrominance Huffman table.
*/
@@ -93,7 +95,7 @@
0xe3, 0xe4, 0xe5, 0xe6, 0xe7,
0xe8, 0xe9, 0xea, 0xf2, 0xf3,
0xf4, 0xf5, 0xf6, 0xf7, 0xf8,
- 0xf9, 0xfa });
+ 0xf9, 0xfa }, false);
/**
* The standard JPEG AC luminance Huffman table.
@@ -133,7 +135,7 @@
0xe4, 0xe5, 0xe6, 0xe7, 0xe8,
0xe9, 0xea, 0xf1, 0xf2, 0xf3,
0xf4, 0xf5, 0xf6, 0xf7, 0xf8,
- 0xf9, 0xfa });
+ 0xf9, 0xfa }, false);
/**
* The standard JPEG DC chrominance Huffman table.
@@ -142,7 +144,7 @@
new JPEGHuffmanTable(new short[] { 0, 3, 1, 1, 1, 1, 1, 1, 1, 1,
1, 0, 0, 0, 0, 0 },
new short[] { 0, 1, 2, 3, 4, 5, 6, 7, 8, 9,
- 10, 11 });
+ 10, 11 }, false);
/**
* The standard JPEG DC luminance Huffman table.
@@ -151,7 +153,7 @@
new JPEGHuffmanTable(new short[] { 0, 1, 5, 1, 1, 1, 1, 1, 1, 0,
0, 0, 0, 0, 0, 0 },
new short[] { 0, 1, 2, 3, 4, 5, 6, 7, 8, 9,
- 10, 11 });
+ 10, 11 }, false);
/**
* Construct and initialize a Huffman table. Copies are created of
@@ -168,9 +170,28 @@
*/
public JPEGHuffmanTable(short[] lengths, short[] values)
{
- if (lengths == null || values == null || lengths.length > 16
- || values.length > 256)
- throw new IllegalArgumentException("invalid length or values array");
+ // Create copies of the lengths and values arguments.
+ this(checkLengths(lengths), checkValues(values, lengths), true);
+ }
+
+ /**
+ * Private constructor that avoids unnecessary copying and argument
+ * checking.
+ *
+ * @param lengths an array of Huffman code lengths
+ * @param values a sorted array of Huffman values
+ * @param copy true if copies should be created of the given arrays
+ */
+ private JPEGHuffmanTable(short[] lengths, short[] values, boolean copy)
+ {
+ this.lengths = copy ? (short[]) lengths.clone() : lengths;
+ this.values = copy ? (short[]) values.clone() : values;
+ }
+
+ private static short[] checkLengths(short[] lengths)
+ {
+ if (lengths == null || lengths.length > 16)
+ throw new IllegalArgumentException("invalid length array");
for (int i = 0; i < lengths.length; i++)
{
@@ -178,12 +199,6 @@
throw new IllegalArgumentException("negative length");
}
- for (int i = 0; i < values.length; i++)
- {
- if (values[i] < 0)
- throw new IllegalArgumentException("negative value");
- }
-
int sum = 0;
for (int i = 0; i < lengths.length; i++)
{
@@ -192,15 +207,30 @@
+ " for code length " + (i + 1));
sum += lengths[i];
}
+
+ return lengths;
+ }
+
+ private static short[] checkValues(short[] values, short[] lengths)
+ {
+ if (values == null || values.length > 256)
+ throw new IllegalArgumentException("invalid values array");
+
+ for (int i = 0; i < values.length; i++)
+ {
+ if (values[i] < 0)
+ throw new IllegalArgumentException("negative value");
+ }
+ // lengths is known-valid by this point.
+ int sum = 0;
+ for (int i = 0; i < lengths.length; i++)
+ sum += lengths[i];
+
if (values.length != sum)
throw new IllegalArgumentException("invalid number of values"
+ " for number of codes");
- this.lengths = new short[lengths.length];
- System.arraycopy(lengths, 0, this.lengths, 0, lengths.length);
-
- this.values = new short[values.length];
- System.arraycopy(values, 0, this.values, 0, values.length);
+ return values;
}
/**
@@ -212,9 +242,7 @@
*/
public short[] getLengths()
{
- short[] lengthsCopy = new short[lengths.length];
- System.arraycopy(lengths, 0, lengthsCopy, 0, lengths.length);
- return lengthsCopy;
+ return (short[]) lengths.clone();
}
/**
@@ -225,9 +253,7 @@
*/
public short[] getValues()
{
- short[] valuesCopy = new short[values.length];
- System.arraycopy(values, 0, valuesCopy, 0, values.length);
- return valuesCopy;
+ return (short[]) values.clone();
}
/**
Index: javax/imageio/plugins/jpeg/JPEGQTable.java
===================================================================
RCS file: /sources/classpath/classpath/javax/imageio/plugins/jpeg/JPEGQTable.java,v
retrieving revision 1.1
diff -u -r1.1 JPEGQTable.java
--- javax/imageio/plugins/jpeg/JPEGQTable.java 31 Mar 2006 02:07:31 -0000 1.1
+++ javax/imageio/plugins/jpeg/JPEGQTable.java 31 Mar 2006 21:50:01 -0000
@@ -66,7 +66,7 @@
24, 35, 55, 64, 81, 104, 113, 92,
49, 64, 78, 87, 103, 121, 120, 101,
72, 92, 95, 98, 112, 100, 103, 99
- });
+ }, false);
/**
* The standard JPEG luminance quantization table, scaled by
@@ -89,7 +89,7 @@
99, 99, 99, 99, 99, 99, 99, 99,
99, 99, 99, 99, 99, 99, 99, 99,
99, 99, 99, 99, 99, 99, 99, 99
- });
+ }, false);
/**
* The standard JPEG chrominance quantization table, scaled by
@@ -99,20 +99,37 @@
K2Chrominance.getScaledInstance(0.5f, true);
/**
- * Construct a new JPEG quantization table.
+ * Construct a new JPEG quantization table. A copy is created of
+ * the table argument.
*
- * @param table the 64-element value table, stored in natural order.
+ * @param table the 64-element value table, stored in natural order
*
* @throws IllegalArgumentException if the table is null or if
* table's length is not equal to 64.
*/
public JPEGQTable(int[] table)
{
+ this(checkTable(table), true);
+ }
+
+ /**
+ * Private constructor that avoids unnecessary copying and argument
+ * checking.
+ *
+ * @param table the 64-element value table, stored in natural order
+ * @param copy true if a copy should be created of the given table
+ */
+ private JPEGQTable(int[] table, boolean copy)
+ {
+ this.table = copy ? (int[]) table.clone() : table;
+ }
+
+ private static int[] checkTable(int[] table)
+ {
if (table == null || table.length != 64)
throw new IllegalArgumentException("invalid JPEG quantization table");
- this.table = new int[table.length];
- System.arraycopy(table, 0, this.table, 0, table.length);
+ return table;
}
/**
@@ -122,9 +139,7 @@
*/
public int[] getTable()
{
- int[] tableCopy = new int[table.length];
- System.arraycopy(table, 0, tableCopy, 0, table.length);
- return tableCopy;
+ return (int[]) table.clone();
}
/**
@@ -153,7 +168,9 @@
scaledTable[i] = max;
}
- return new JPEGQTable(scaledTable);
+ // Do not copy scaledTable. It is already a copy because we used
+ // getTable to retrieve it.
+ return new JPEGQTable(scaledTable, false);
}
/**
