Skip to content

Commit e49e5b5

Browse files
committed
8273972: Multi-core choke point in CMM engine (LCMSTransform.doTransform)
Reviewed-by: prr
1 parent 2072bc7 commit e49e5b5

5 files changed

Lines changed: 333 additions & 41 deletions

File tree

src/java.desktop/share/classes/sun/java2d/cmm/lcms/LCMS.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -148,7 +148,7 @@ public synchronized ColorTransform createTransform(
148148
}
149149

150150
/* methods invoked from LCMSTransform */
151-
public static native void colorConvert(LCMSTransform trans,
151+
public static native void colorConvert(long trans,
152152
LCMSImageLayout src,
153153
LCMSImageLayout dest);
154154

src/java.desktop/share/classes/sun/java2d/cmm/lcms/LCMSTransform.java

Lines changed: 41 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -44,18 +44,30 @@
4444
import java.awt.image.Raster;
4545
import java.awt.image.SampleModel;
4646
import java.awt.image.WritableRaster;
47+
import java.lang.ref.Reference;
4748

4849
import sun.java2d.cmm.ColorTransform;
4950

5051
import static sun.java2d.cmm.lcms.LCMSImageLayout.ImageLayoutException;
5152

5253
final class LCMSTransform implements ColorTransform {
53-
long ID;
54-
private int inFormatter = 0;
55-
private boolean isInIntPacked = false;
56-
private int outFormatter = 0;
57-
private boolean isOutIntPacked = false;
5854

55+
private final static class NativeTransform {
56+
private long ID;
57+
private int inFormatter;
58+
private boolean isInIntPacked;
59+
private int outFormatter;
60+
private boolean isOutIntPacked;
61+
62+
private boolean match(LCMSImageLayout in, LCMSImageLayout out) {
63+
return inFormatter == in.pixelType
64+
&& isInIntPacked == in.isIntPacked
65+
&& outFormatter == out.pixelType
66+
&& isOutIntPacked == out.isIntPacked;
67+
}
68+
}
69+
70+
private volatile NativeTransform transform;
5971
ICC_Profile[] profiles;
6072
LCMSProfile[] lcmsProfiles;
6173
int renderType;
@@ -64,8 +76,6 @@ final class LCMSTransform implements ColorTransform {
6476
private int numInComponents = -1;
6577
private int numOutComponents = -1;
6678

67-
private Object disposerReferent = new Object();
68-
6979
public LCMSTransform(ICC_Profile profile, int renderType,
7080
int transformType)
7181
{
@@ -122,31 +132,32 @@ public int getNumOutComponents() {
122132
return numOutComponents;
123133
}
124134

125-
private synchronized void doTransform(LCMSImageLayout in,
126-
LCMSImageLayout out) {
127-
// update native transfrom if needed
128-
if (ID == 0L ||
129-
inFormatter != in.pixelType || isInIntPacked != in.isIntPacked ||
130-
outFormatter != out.pixelType || isOutIntPacked != out.isIntPacked)
131-
{
132-
133-
if (ID != 0L) {
134-
// Disposer will destroy forgotten transform
135-
disposerReferent = new Object();
135+
private void doTransform(LCMSImageLayout in, LCMSImageLayout out) {
136+
NativeTransform tfm = transform;
137+
// update native transform if needed
138+
if (tfm == null || !tfm.match(in, out)) {
139+
synchronized (this) {
140+
tfm = transform;
141+
if (tfm == null || !tfm.match(in, out)) {
142+
tfm = new NativeTransform();
143+
tfm.inFormatter = in.pixelType;
144+
tfm.isInIntPacked = in.isIntPacked;
145+
146+
tfm.outFormatter = out.pixelType;
147+
tfm.isOutIntPacked = out.isIntPacked;
148+
149+
tfm.ID = LCMS.createTransform(lcmsProfiles, renderType,
150+
tfm.inFormatter,
151+
tfm.isInIntPacked,
152+
tfm.outFormatter,
153+
tfm.isOutIntPacked, tfm);
154+
// Disposer will destroy forgotten transform
155+
transform = tfm;
156+
}
136157
}
137-
inFormatter = in.pixelType;
138-
isInIntPacked = in.isIntPacked;
139-
140-
outFormatter = out.pixelType;
141-
isOutIntPacked = out.isIntPacked;
142-
143-
ID = LCMS.createTransform(lcmsProfiles, renderType,
144-
inFormatter, isInIntPacked,
145-
outFormatter, isOutIntPacked,
146-
disposerReferent);
147158
}
148-
149-
LCMS.colorConvert(this, in, out);
159+
LCMS.colorConvert(tfm.ID, in, out);
160+
Reference.reachabilityFence(tfm); // prevent deallocation of "tfm.ID"
150161
}
151162

152163
/**

src/java.desktop/share/native/liblcms/LCMS.c

Lines changed: 2 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,6 @@ typedef union {
7171
} TagSignature_t, *TagSignature_p;
7272

7373
static jfieldID Trans_renderType_fID;
74-
static jfieldID Trans_ID_fID;
7574
static jfieldID IL_isIntPacked_fID;
7675
static jfieldID IL_dataType_fID;
7776
static jfieldID IL_pixelType_fID;
@@ -510,9 +509,9 @@ void releaseILData (JNIEnv *env, void* pData, jint dataType,
510509
* Signature: (Lsun/java2d/cmm/lcms/LCMSTransform;Lsun/java2d/cmm/lcms/LCMSImageLayout;Lsun/java2d/cmm/lcms/LCMSImageLayout;)V
511510
*/
512511
JNIEXPORT void JNICALL Java_sun_java2d_cmm_lcms_LCMS_colorConvert
513-
(JNIEnv *env, jclass cls, jobject trans, jobject src, jobject dst)
512+
(JNIEnv *env, jclass cls, jlong ID, jobject src, jobject dst)
514513
{
515-
cmsHTRANSFORM sTrans = NULL;
514+
cmsHTRANSFORM sTrans = jlong_to_ptr(ID);
516515
int srcDType, dstDType;
517516
int srcOffset, srcNextRowOffset, dstOffset, dstNextRowOffset;
518517
int width, height, i;
@@ -533,8 +532,6 @@ JNIEXPORT void JNICALL Java_sun_java2d_cmm_lcms_LCMS_colorConvert
533532
srcAtOnce = (*env)->GetBooleanField(env, src, IL_imageAtOnce_fID);
534533
dstAtOnce = (*env)->GetBooleanField(env, dst, IL_imageAtOnce_fID);
535534

536-
sTrans = jlong_to_ptr((*env)->GetLongField (env, trans, Trans_ID_fID));
537-
538535
if (sTrans == NULL) {
539536
J2dRlsTraceLn(J2D_TRACE_ERROR, "LCMS_colorConvert: transform == NULL");
540537
JNU_ThrowByName(env, "java/awt/color/CMMException",
@@ -626,11 +623,6 @@ JNIEXPORT void JNICALL Java_sun_java2d_cmm_lcms_LCMS_initLCMS
626623
if (Trans_renderType_fID == NULL) {
627624
return;
628625
}
629-
Trans_ID_fID = (*env)->GetFieldID (env, Trans, "ID", "J");
630-
if (Trans_ID_fID == NULL) {
631-
return;
632-
}
633-
634626
IL_isIntPacked_fID = (*env)->GetFieldID (env, IL, "isIntPacked", "Z");
635627
if (IL_isIntPacked_fID == NULL) {
636628
return;
Lines changed: 146 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,146 @@
1+
/*
2+
* Copyright Amazon.com Inc. or its affiliates. All Rights Reserved.
3+
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
4+
*
5+
* This code is free software; you can redistribute it and/or modify it
6+
* under the terms of the GNU General Public License version 2 only, as
7+
* published by the Free Software Foundation.
8+
*
9+
* This code is distributed in the hope that it will be useful, but WITHOUT
10+
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
11+
* FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
12+
* version 2 for more details (a copy is included in the LICENSE file that
13+
* accompanied this code).
14+
*
15+
* You should have received a copy of the GNU General Public License version
16+
* 2 along with this work; if not, write to the Free Software Foundation,
17+
* Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
18+
*
19+
* Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
20+
* or visit www.oracle.com if you need additional information or have any
21+
* questions.
22+
*/
23+
24+
import java.awt.color.ColorSpace;
25+
import java.awt.image.BufferedImage;
26+
import java.awt.image.ColorConvertOp;
27+
28+
/**
29+
* @test
30+
* @bug 8273972
31+
* @summary Verifies that ColorConvertOp works fine if shared between threads
32+
* @run main/othervm/timeout=600 MTTransformValidation
33+
*/
34+
public final class MTPerLineTransformValidation {
35+
36+
private volatile static BufferedImage[] lines;
37+
38+
public static final int SIZE = 255;
39+
private static volatile boolean failed = false;
40+
41+
private static final int[] spaces = {
42+
ColorSpace.CS_CIEXYZ, ColorSpace.CS_GRAY, ColorSpace.CS_LINEAR_RGB,
43+
ColorSpace.CS_PYCC, ColorSpace.CS_sRGB
44+
};
45+
46+
private static final int[] types = new int[]{
47+
BufferedImage.TYPE_INT_RGB, BufferedImage.TYPE_INT_ARGB,
48+
BufferedImage.TYPE_INT_ARGB_PRE, BufferedImage.TYPE_INT_BGR,
49+
BufferedImage.TYPE_3BYTE_BGR, BufferedImage.TYPE_4BYTE_ABGR,
50+
BufferedImage.TYPE_4BYTE_ABGR_PRE,
51+
BufferedImage.TYPE_USHORT_565_RGB,
52+
BufferedImage.TYPE_USHORT_555_RGB, BufferedImage.TYPE_BYTE_GRAY,
53+
BufferedImage.TYPE_USHORT_GRAY, BufferedImage.TYPE_BYTE_BINARY,
54+
BufferedImage.TYPE_BYTE_INDEXED
55+
};
56+
57+
/**
58+
* For all possible combinations of color spaces and image types, convert
59+
* the source image using one shared ColorConvertOp per line on the
60+
* different threads. The result is validated against images converted on
61+
* one thread only.
62+
*/
63+
public static void main(String[] args) throws Exception {
64+
for (int srcCS : spaces) {
65+
for (int dstCS : spaces) {
66+
if(srcCS != dstCS) {
67+
for (int type : types) {
68+
checkTypes(ColorSpace.getInstance(srcCS),
69+
ColorSpace.getInstance(dstCS), type);
70+
}
71+
}
72+
}
73+
}
74+
}
75+
76+
private static void checkTypes(ColorSpace srcCS, ColorSpace dstCS, int type)
77+
throws Exception {
78+
lines = new BufferedImage[SIZE];
79+
ColorConvertOp goldOp = new ColorConvertOp(srcCS, dstCS, null);
80+
BufferedImage src = createSrc(type);
81+
BufferedImage gold = goldOp.filter(src, null);
82+
83+
// we do not share the goldOp since it is already initialized and used
84+
// for the whole image, instead we will create a separate sharedOp and
85+
// use it for each line of a different threads
86+
ColorConvertOp sharedOp = new ColorConvertOp(srcCS, dstCS, null);
87+
Thread[] threads = new Thread[SIZE];
88+
for (int y = 0; y < SIZE; ++y) {
89+
BufferedImage line = src.getSubimage(0, y, SIZE, 1);
90+
threads[y] = test(sharedOp, line, y);
91+
}
92+
93+
for (Thread t: threads) {
94+
t.start();
95+
}
96+
for (Thread t: threads) {
97+
t.join();
98+
}
99+
for (int y = 0; y < SIZE; ++y) {
100+
validate(gold, lines[y], y);
101+
}
102+
if (failed) {
103+
throw new RuntimeException("Unexpected exception");
104+
}
105+
}
106+
107+
private static Thread test(ColorConvertOp sharedOp,
108+
BufferedImage line, int y){
109+
return new Thread(() -> {
110+
try {
111+
BufferedImage image = sharedOp.filter(line, null);
112+
lines[y] = image;
113+
} catch (Throwable t) {
114+
t.printStackTrace();
115+
failed = true;
116+
}
117+
});
118+
}
119+
120+
private static BufferedImage createSrc(int type) {
121+
BufferedImage img = new BufferedImage(SIZE, SIZE, type);
122+
fill(img);
123+
return img;
124+
}
125+
126+
private static void fill(BufferedImage image) {
127+
for (int i = 0; i < SIZE; i++) {
128+
for (int j = 0; j < SIZE; j++) {
129+
image.setRGB(i, j,
130+
(i << 24) | (i << 16) | (j << 8) | ((i + j) >> 1));
131+
}
132+
}
133+
}
134+
135+
private static void validate(BufferedImage full, BufferedImage line, int y) {
136+
for (int i = 0; i < SIZE; i++) {
137+
int rgb1 = full.getRGB(i, y);
138+
int rgb2 = line.getRGB(i, 0);
139+
if (rgb1 != rgb2) {
140+
System.err.println("rgb1 = " + Integer.toHexString(rgb1));
141+
System.err.println("rgb2 = " + Integer.toHexString(rgb2));
142+
throw new RuntimeException();
143+
}
144+
}
145+
}
146+
}

0 commit comments

Comments
 (0)