Skip to content

Commit 63cedaf

Browse files
jerboaajerboaa
committed
8347334: JimageDiffGenerator code clean-ups
Reviewed-by: henryjen, mbaesken
1 parent 254e840 commit 63cedaf

File tree

2 files changed

+340
-27
lines changed

2 files changed

+340
-27
lines changed

‎src/jdk.jlink/share/classes/jdk/tools/jlink/internal/runtimelink/JimageDiffGenerator.java

+13-27
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2024, Red Hat, Inc.
2+
* Copyright (c) 2024, 2025, Red Hat, Inc.
33
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
44
*
55
* This code is free software; you can redistribute it and/or modify it
@@ -126,39 +126,25 @@ public List<ResourceDiff> generateDiff(ImageResource base, ImageResource image)
126126
private boolean compareStreams(InputStream is1, InputStream is2) {
127127
byte[] buf1 = new byte[1024];
128128
byte[] buf2 = new byte[1024];
129-
int bytesRead1, bytesRead2 = 0;
130-
try {
131-
try (is1; is2) {
132-
while ((bytesRead1 = is1.read(buf1)) != -1 &&
133-
(bytesRead2 = is2.read(buf2)) != -1) {
134-
if (bytesRead1 != bytesRead2) {
135-
return false;
136-
}
137-
if (bytesRead1 == buf1.length) {
138-
if (!Arrays.equals(buf1, buf2)) {
139-
return false;
140-
}
141-
} else {
142-
for (int i = 0; i < bytesRead1; i++) {
143-
if (buf1[i] != buf2[i]) {
144-
return false;
145-
}
146-
}
147-
}
129+
int bytesRead1, bytesRead2;
130+
try (is1; is2) {
131+
while (true) {
132+
bytesRead1 = is1.readNBytes(buf1, 0, buf1.length);
133+
bytesRead2 = is2.readNBytes(buf2, 0, buf2.length);
134+
if (!Arrays.equals(buf1, 0, bytesRead1,
135+
buf2, 0, bytesRead2)) {
136+
return false;
148137
}
149-
// ensure we read both to the end
150-
if (bytesRead1 == -1) {
151-
bytesRead2 = is2.read(buf2);
152-
if (bytesRead2 != -1) {
153-
return false;
154-
}
138+
if (bytesRead1 == 0) {
139+
// If we reach here, bytesRead2 must be 0 as well, otherwise
140+
// we return false on the !Arrays.equals() check above.
141+
assert bytesRead2 == 0 : "Arrays must have been read to the end";
155142
return true;
156143
}
157144
}
158145
} catch (IOException e) {
159146
throw new UncheckedIOException("IO exception when comparing bytes", e);
160147
}
161-
return false;
162148
}
163149

164150
}

‎test/jdk/tools/jlink/runtimeImage/JimageDiffGeneratorTest.java

+327
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,327 @@
1+
/*
2+
* Copyright (c) 2025, Red Hat, Inc.
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 static org.testng.Assert.assertEquals;
25+
import static org.testng.Assert.assertTrue;
26+
27+
import java.io.ByteArrayInputStream;
28+
import java.io.InputStream;
29+
import java.util.List;
30+
31+
import org.testng.annotations.Test;
32+
33+
import jdk.tools.jlink.internal.runtimelink.JimageDiffGenerator;
34+
import jdk.tools.jlink.internal.runtimelink.JimageDiffGenerator.ImageResource;
35+
import jdk.tools.jlink.internal.runtimelink.ResourceDiff;
36+
37+
/*
38+
* @test
39+
* @summary Unit test the diff generator logic for JEP 493
40+
* @modules java.base/jdk.internal.jimage
41+
* jdk.jlink/jdk.tools.jlink.internal.runtimelink
42+
* @run testng JimageDiffGeneratorTest
43+
*/
44+
public class JimageDiffGeneratorTest {
45+
46+
/*
47+
* Expect a resource diff since the "b" item is removed in
48+
* the optimized image.
49+
*/
50+
@Test
51+
public void testItemsRemovedInOpt() throws Exception {
52+
List<String> entriesOpt = List.of("a", "c", "d");
53+
byte[][] bytesOpt = new byte[][] {
54+
{ 0x01, 0x03, 0x03 }, /* a */
55+
{ 0x09, 0x11, 0x11 }, /* c */
56+
{ 0x22, 0x22, 0x30 }, /* d */
57+
};
58+
ImageResource opt = new BasicImageResource(entriesOpt, bytesOpt);
59+
List<String> entriesBase = List.of("a", "b", "c", "d");
60+
byte[][] bytesBase = new byte[][] {
61+
{ 0x01, 0x03, 0x03 }, /* a */
62+
{ 0x08, 0x04, 0x04 }, /* b */
63+
{ 0x09, 0x11, 0x11 }, /* c */
64+
{ 0x22, 0x22, 0x30 }, /* d */
65+
};
66+
ImageResource base = new BasicImageResource(entriesBase, bytesBase);
67+
JimageDiffGenerator gen = new JimageDiffGenerator();
68+
List<ResourceDiff> result = gen.generateDiff(base, opt);
69+
assertEquals(result.size(), 1);
70+
assertEquals(result.get(0).getKind(), ResourceDiff.Kind.REMOVED);
71+
assertEquals(result.get(0).getName(), "b");
72+
assertEquals(result.get(0).getResourceBytes(), bytesBase[1]);
73+
}
74+
75+
/*
76+
* Expect no difference as streams are the same
77+
*/
78+
@Test
79+
public void testNoDiff() throws Exception {
80+
List<String> entriesBase = List.of("a", "b", "c", "d");
81+
byte[][] bytesBase = new byte[][] {
82+
{ 0x01, 0x03, 0x03 }, /* a */
83+
{ 0x08, 0x04, 0x04 }, /* b */
84+
{ 0x09, 0x11, 0x11 }, /* c */
85+
{ 0x22, 0x22, 0x30 }, /* d */
86+
};
87+
ImageResource base = new BasicImageResource(entriesBase, bytesBase);
88+
ImageResource opt = new BasicImageResource(entriesBase, bytesBase);
89+
JimageDiffGenerator gen = new JimageDiffGenerator();
90+
List<ResourceDiff> result = gen.generateDiff(base, opt);
91+
assertTrue(result.isEmpty());
92+
}
93+
94+
/*
95+
* Expect a resource diff since the "b" item has been added in
96+
* the optimized image.
97+
*/
98+
@Test
99+
public void testItemsAddedInOpt() throws Exception {
100+
List<String> entriesBase = List.of("a", "c", "d");
101+
byte[][] bytesBase = new byte[][] {
102+
{ 0x01, 0x03, 0x03 }, /* a */
103+
{ 0x09, 0x11, 0x11 }, /* c */
104+
{ 0x22, 0x22, 0x30 }, /* d */
105+
};
106+
ImageResource base = new BasicImageResource(entriesBase, bytesBase);
107+
List<String> entriesOpt = List.of("a", "b", "c", "d");
108+
byte[][] bytesOpt = new byte[][] {
109+
{ 0x01, 0x03, 0x03 }, /* a */
110+
{ 0x08, 0x04, 0x04 }, /* b */
111+
{ 0x09, 0x11, 0x11 }, /* c */
112+
{ 0x22, 0x22, 0x30 }, /* d */
113+
};
114+
ImageResource opt = new BasicImageResource(entriesOpt, bytesOpt);
115+
JimageDiffGenerator gen = new JimageDiffGenerator();
116+
List<ResourceDiff> result = gen.generateDiff(base, opt);
117+
assertEquals(result.size(), 1);
118+
assertEquals(result.get(0).getKind(), ResourceDiff.Kind.ADDED);
119+
assertEquals(result.get(0).getName(), "b");
120+
assertEquals(result.get(0).getResourceBytes(), null, "Added entries in opt don't have resource bytes");
121+
}
122+
123+
/*
124+
* Expect a resource diff since the "d" item has modified bytes in the
125+
* optimized image resource.
126+
*/
127+
@Test
128+
public void testBytesDiffer() throws Exception {
129+
List<String> entriesBase = List.of("a", "b", "c", "d");
130+
byte[][] bytesBase = new byte[][] {
131+
{ 0x01, 0x03, 0x03 }, /* a */
132+
{ 0x08, 0x04, 0x04 }, /* b */
133+
{ 0x09, 0x11, 0x11 }, /* c */
134+
{ 0x11, 0x12, 0x31 }, /* d */
135+
};
136+
ImageResource base = new BasicImageResource(entriesBase, bytesBase);
137+
List<String> entriesOpt = List.of("a", "b", "c", "d");
138+
byte[][] bytesOpt = new byte[][] {
139+
{ 0x01, 0x03, 0x03 }, /* a */
140+
{ 0x08, 0x04, 0x04 }, /* b */
141+
{ 0x09, 0x11, 0x11 }, /* c */
142+
{ 0x22, 0x22, 0x30 }, /* d - differs to base! */
143+
};
144+
ImageResource opt = new BasicImageResource(entriesOpt, bytesOpt);
145+
JimageDiffGenerator gen = new JimageDiffGenerator();
146+
List<ResourceDiff> result = gen.generateDiff(base, opt);
147+
assertEquals(result.size(), 1);
148+
assertEquals(result.get(0).getKind(), ResourceDiff.Kind.MODIFIED);
149+
assertEquals(result.get(0).getName(), "d");
150+
assertEquals(result.get(0).getResourceBytes(), bytesBase[3]);
151+
}
152+
153+
/*
154+
* Expect a resource diff since an item has modified bytes. Test
155+
* for a resource that has more than 1K bytes (the buffer size used
156+
* internally).
157+
*/
158+
@Test
159+
public void testBytesDifferLarge() throws Exception {
160+
List<String> entriesBase = List.of("a", "b", "c", "d");
161+
byte[][] bytesBase = new byte[][] {
162+
{ 0x01, 0x03, 0x03 }, /* a */
163+
{ 0x08, 0x04, 0x04 }, /* b */
164+
{ }, /* c */
165+
{ 0x11, 0x12, 0x31 }, /* d */
166+
};
167+
bytesBase[2] = generateBytes();
168+
ImageResource base = new BasicImageResource(entriesBase, bytesBase);
169+
List<String> entriesOpt = List.of("a", "b", "c", "d");
170+
byte[][] bytesOpt = new byte[][] {
171+
{ 0x01, 0x03, 0x03 }, /* a */
172+
{ 0x08, 0x04, 0x04 }, /* b */
173+
{ }, /* c */
174+
{ 0x22, 0x22, 0x30 }, /* d */
175+
};
176+
bytesOpt[2] = generateBytes();
177+
// Change the first byte of 'c' in the opt bytes
178+
bytesOpt[2][0] = -1;
179+
// assert pre-condition
180+
assertTrue(bytesOpt[2][0] != bytesBase[2][0]);
181+
182+
ImageResource opt = new BasicImageResource(entriesOpt, bytesOpt);
183+
JimageDiffGenerator gen = new JimageDiffGenerator();
184+
List<ResourceDiff> result = gen.generateDiff(base, opt);
185+
assertEquals(result.size(), 2);
186+
// assertions for 'c' differences
187+
assertEquals(result.get(0).getKind(), ResourceDiff.Kind.MODIFIED);
188+
assertEquals(result.get(0).getName(), "c");
189+
assertEquals(result.get(0).getResourceBytes(), bytesBase[2]);
190+
191+
// assertion for 'd' differences
192+
assertEquals(result.get(1).getKind(), ResourceDiff.Kind.MODIFIED);
193+
assertEquals(result.get(1).getName(), "d");
194+
assertEquals(result.get(1).getResourceBytes(), bytesBase[3]);
195+
}
196+
197+
/*
198+
* Expect a no resource difference since the steams are both empty
199+
*/
200+
@Test
201+
public void testEmptyStreams() throws Exception {
202+
List<String> entriesBase = List.of("a", "b", "c", "d");
203+
byte[][] bytesBase = new byte[][] {
204+
{ }, /* a */
205+
{ }, /* b */
206+
{ }, /* c */
207+
{ }, /* d */
208+
};
209+
ImageResource base = new BasicImageResource(entriesBase, bytesBase);
210+
ImageResource opt = new BasicImageResource(entriesBase, bytesBase);
211+
JimageDiffGenerator gen = new JimageDiffGenerator();
212+
List<ResourceDiff> result = gen.generateDiff(base, opt);
213+
assertTrue(result.isEmpty());
214+
}
215+
216+
/*
217+
* Expect a difference since entry 'a' has zero bytes in opt.
218+
*/
219+
@Test
220+
public void testNotEqualLength() throws Exception {
221+
List<String> entriesBase = List.of("a", "b", "c", "d");
222+
byte[][] bytesBase = new byte[][] {
223+
{ 0x01, 0x03, 0x03 }, /* a */
224+
{ 0x08, 0x04, 0x04 }, /* b */
225+
{ 0x09, 0x11, 0x11 }, /* c */
226+
{ 0x11, 0x12, 0x31 }, /* d */
227+
};
228+
byte[][] bytesOpt = new byte[][] {
229+
{ }, /* a */
230+
{ 0x08, 0x04, 0x04 }, /* b */
231+
{ 0x09, 0x11, 0x11 }, /* c */
232+
{ 0x11, 0x12, 0x31 }, /* d */
233+
};
234+
ImageResource base = new BasicImageResource(entriesBase, bytesBase);
235+
ImageResource opt = new BasicImageResource(entriesBase, bytesOpt);
236+
JimageDiffGenerator gen = new JimageDiffGenerator();
237+
List<ResourceDiff> result = gen.generateDiff(base, opt);
238+
assertEquals(result.size(), 1);
239+
assertEquals(result.get(0).getKind(), ResourceDiff.Kind.MODIFIED);
240+
assertEquals(result.get(0).getName(), "a");
241+
assertEquals(result.get(0).getResourceBytes(), bytesBase[0]);
242+
}
243+
244+
/*
245+
* Expect a difference since entry 'a' on the optimized version is
246+
* one byte longer.
247+
*/
248+
@Test
249+
public void testBytesDifferExactBufferSize() throws Exception {
250+
List<String> entriesBase = List.of("a", "b", "c", "d");
251+
byte[][] bytesBase = new byte[][] {
252+
{ }, /* a */
253+
{ 0x08, 0x04, 0x04 }, /* b */
254+
{ 0x09, 0x11, 0x11 }, /* c */
255+
{ 0x11, 0x12, 0x31 }, /* d */
256+
};
257+
byte[][] bytesOpt = new byte[][] {
258+
{ }, /* a */
259+
{ 0x08, 0x04, 0x04 }, /* b */
260+
{ 0x09, 0x11, 0x11 }, /* c */
261+
{ 0x11, 0x12, 0x31 }, /* d */
262+
};
263+
bytesBase[0] = genBytesOfSize(1024); // exact buffer size
264+
bytesOpt[0] = genBytesOfSize(1024 + 1); // buffer size + 1
265+
266+
ImageResource base = new BasicImageResource(entriesBase, bytesBase);
267+
ImageResource opt = new BasicImageResource(entriesBase, bytesOpt);
268+
JimageDiffGenerator gen = new JimageDiffGenerator();
269+
List<ResourceDiff> result = gen.generateDiff(base, opt);
270+
assertEquals(result.size(), 1);
271+
assertEquals(result.get(0).getKind(), ResourceDiff.Kind.MODIFIED);
272+
assertEquals(result.get(0).getName(), "a");
273+
assertEquals(result.get(0).getResourceBytes(), bytesBase[0]);
274+
}
275+
276+
private byte[] generateBytes() {
277+
int size = 1024 + 254;
278+
return genBytesOfSize(size);
279+
}
280+
281+
private byte[] genBytesOfSize(int size) {
282+
byte[] result = new byte[size];
283+
for (int i = 0; i < size; i++) {
284+
result[i] = (byte)(i % Byte.MAX_VALUE);
285+
}
286+
return result;
287+
}
288+
289+
// Simple stub ImageResource for test purposes
290+
static class BasicImageResource implements ImageResource {
291+
292+
private final List<String> entries;
293+
private final byte[][] entryBytes;
294+
295+
public BasicImageResource(List<String> entries, byte[][] entryBytes) {
296+
this.entries = entries;
297+
this.entryBytes = entryBytes;
298+
}
299+
300+
@Override
301+
public void close() throws Exception {
302+
// nothing
303+
}
304+
305+
@Override
306+
public List<String> getEntries() {
307+
return entries;
308+
}
309+
310+
@Override
311+
public byte[] getResourceBytes(String name) {
312+
for (int i = 0; i < entries.size(); i++) {
313+
if (entries.get(i).equals(name)) {
314+
return entryBytes[i];
315+
}
316+
}
317+
return null;
318+
}
319+
320+
@Override
321+
public InputStream getResource(String name) {
322+
byte[] bytes = getResourceBytes(name);
323+
return new ByteArrayInputStream(bytes);
324+
}
325+
326+
}
327+
}

0 commit comments

Comments
 (0)