RFR: 8358623: Avoid unnecessary data copying in ICC_Profile

[Harshitha Onkar]

On Wed, 4 Jun 2025 23:39:05 GMT, Sergey Bylokhov <serb at openjdk.org> wrote:

> This PR simplifies several aspects of the ICC_Profile class:
> 
> - [Change 1](https://github.com/openjdk/jdk/pull/25650/commits/426a608b1df9e39e221d05e7374a3fecf6e6cf30):
>     The ICC_Profile.getInstance(byte[] data) method used to copy the profile header for validation. This copy appears redundant, as the original data array is used later anyway. This logic was originally introduced by [JDK-8347377](https://bugs.openjdk.org/browse/JDK-8347377).
> 
> - [Change 2](https://github.com/openjdk/jdk/pull/25650/commits/4035c8b1f7e1dcbc9941ead939218bba47b0a2fe):
>     In some places, the code retrieves the profile header using getData(icSigHead), which always creates a new array. It is now replaced with private getData(cmmProfile(), icSigHead) to avoid unnecessary copying. To clarify the purpose of the private method, I have added documentation.
> 
>  - [Change 3](https://github.com/openjdk/jdk/pull/25650/commits/96ad456593de3dd68c3ae6840fffee7bac68bc0c):
> After Change 2, static analysis tools began reporting a potential NPE when using getData(cmmProfile(), icSigHead), since it may return null. To address this, the internal implementation of getData was updated to always return a non-null value or throw an exception. The public method now catches this exception and returns null, as required by the specification. **Note**: this potential NPE is not a regression introduced by any changes, it simply became easier for tools to detect due to the simplified code.
> 
> @prrace @honkar-jdk please take a look

src/java.desktop/share/classes/java/awt/color/ICC_Profile.java line 802:

> 800:     public static ICC_Profile getInstance(byte[] data) {
> 801:         ProfileDataVerifier.verify(data);
> 802:         verifyHeader(data);

[verifyHeader(byte[] data)](https://github.com/openjdk/jdk/blob/3b32f6a8ec37338764d3e6713247ff96e49bf5b3/src/java.desktop/share/classes/java/awt/color/ICC_Profile.java#L1176C2-L1184C6), expects header byte array and not the entire profile array, is it technically correct to send the entire profile byte array?

Please note verifyHeader() is called from two places from `getInstance(byte[] data)` and `setData(int tagSignature, byte[] tagData)`, 

With the current fix, logically all the checks within verifyHeader() work (getProfileClass(data), getColorSpaceType(data) ...) because the data is extracted at specified index from the superset - entire profile byte array instead of the header byte array.

If it is decided to send only header byte array to `verifyHeader() ` then changing the method param name to **header** instead of **data** may be clearer.


    private static void verifyHeader(byte[] header) {
        if (header == null || header.length < HEADER_SIZE) {
            throw new IllegalArgumentException("Invalid header data");
        }
        getProfileClass(header);
        getColorSpaceType(header);
        getPCSType(header);
        checkRenderingIntent(header);
    }

src/java.desktop/share/classes/java/awt/color/ICC_Profile.java line 807:

> 805:             System.arraycopy(data, 0, theHeader, 0, HEADER_SIZE);
> 806:             verifyHeader(theHeader);
> 807: 

In reference to the above comment , these lines may need to be reverted as verifyHeader(byte[] header) expects the header array and not the entire profile byte array.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/25650#discussion_r2141743069
PR Review Comment: https://git.openjdk.org/jdk/pull/25650#discussion_r2141747655