Skip to content

Commit c795417

Browse files
dmtrKovalenkodmtrKovalenko
authored
feat: Optimize gc cycles and introduce flag for reduced memory usage (#95)
* feat: Optimize gc cycles and introduce flag for reduced memoryusage * Update readme * Disable ram optimization in debug test * Fix node bindgins tests
1 parent e527f35 commit c795417

10 files changed

+122
-74
lines changed

‎README.md

+2
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,8 @@ export type ODiffOptions = Partial<{
111111
antialiasing: boolean;
112112
/** If `true` reason: "pixel-diff" output will contain the set of line indexes containing different pixels */
113113
captureDiffLines: boolean;
114+
/** If `true` odiff will use less memory but will be slower with larger images */
115+
reduceRamUsage: boolean;
114116
/** An array of regions to ignore in the diff. */
115117
ignoreRegions: Array<{
116118
x1: number;

‎bin/Color.ml

+27-19
Original file line numberDiff line numberDiff line change
@@ -1,23 +1,31 @@
11
let ofHexString s =
22
match String.length s with
3-
| (4 | 7) as len -> (
4-
let short = len = 4 in
5-
let r' =
6-
match short with true -> String.sub s 1 1 | false -> String.sub s 1 2
7-
in
8-
let g' =
9-
match short with true -> String.sub s 2 1 | false -> String.sub s 3 2
10-
in
11-
let b' =
12-
match short with true -> String.sub s 3 1 | false -> String.sub s 5 2
13-
in
14-
let r = int_of_string_opt ("0x" ^ r') in
15-
let g = int_of_string_opt ("0x" ^ g') in
16-
let b = int_of_string_opt ("0x" ^ b') in
3+
| (4 | 7) as len ->
4+
(let short = len = 4 in
5+
let r' =
6+
match short with true -> String.sub s 1 1 | false -> String.sub s 1 2
7+
in
8+
let g' =
9+
match short with true -> String.sub s 2 1 | false -> String.sub s 3 2
10+
in
11+
let b' =
12+
match short with true -> String.sub s 3 1 | false -> String.sub s 5 2
13+
in
14+
let r = int_of_string_opt ("0x" ^ r') in
15+
let g = int_of_string_opt ("0x" ^ g') in
16+
let b = int_of_string_opt ("0x" ^ b') in
1717

18-
match (r, g, b) with
19-
| Some r, Some g, Some b when short ->
20-
Some ((16 * r) + r, (16 * g) + g, (16 * b) + b)
21-
| Some r, Some g, Some b -> Some (r, g, b)
22-
| _ -> None)
18+
match (r, g, b) with
19+
| Some r, Some g, Some b when short ->
20+
Some ((16 * r) + r, (16 * g) + g, (16 * b) + b)
21+
| Some r, Some g, Some b -> Some (r, g, b)
22+
| _ -> None)
23+
|> Option.map (fun (r, g, b) ->
24+
(* Create rgba pixel value right after parsing *)
25+
let r = (r land 255) lsl 0 in
26+
let g = (g land 255) lsl 8 in
27+
let b = (b land 255) lsl 16 in
28+
let a = 255 lsl 24 in
29+
30+
Int32.of_int (a lor b lor g lor r))
2331
| _ -> None

‎bin/Main.ml

+22-4
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,23 @@ type 'output diffResult = { exitCode : int; diff : 'output option }
1414
(* Arguments must remain positional for the cmd parser lib that we use *)
1515
let main img1Path img2Path diffPath threshold outputDiffMask failOnLayoutChange
1616
diffColorHex toEmitStdoutParsableString antialiasing ignoreRegions diffLines
17-
=
17+
disableGcOptimizations =
18+
(*
19+
We do not need to actually maintain memory size of the allocated RAM by odiff, so we are
20+
increasing the minor memory size to avoid most of the possible deallocations. For sure it is
21+
not possible be sure that it won't be run in OCaml because we allocate the Stack and Queue
22+
23+
By default set the minor heap size to 256mb on 64bit machine
24+
*)
25+
if not disableGcOptimizations then
26+
Gc.set
27+
{
28+
(Gc.get ()) with
29+
Gc.minor_heap_size = 64_000_000;
30+
Gc.stack_limit = 2_048_000;
31+
Gc.window_size = 25;
32+
};
33+
1834
let module IO1 = (val getIOModule img1Path) in
1935
let module IO2 = (val getIOModule img2Path) in
2036
let module Diff = MakeDiff (IO1) (IO2) in
@@ -24,9 +40,9 @@ let main img1Path img2Path diffPath threshold outputDiffMask failOnLayoutChange
2440
Diff.diff img1 img2 ~outputDiffMask ~threshold ~failOnLayoutChange
2541
~antialiasing ~ignoreRegions ~diffLines
2642
~diffPixel:
27-
(Color.ofHexString diffColorHex |> function
28-
| Some col -> col
29-
| None -> (255, 0, 0))
43+
(match Color.ofHexString diffColorHex with
44+
| Some c -> c
45+
| None -> redPixel)
3046
()
3147
|> Print.printDiffResult toEmitStdoutParsableString
3248
|> function
@@ -43,4 +59,6 @@ let main img1Path img2Path diffPath threshold outputDiffMask failOnLayoutChange
4359
(match diff with
4460
| Some output when outputDiffMask -> IO1.freeImage output
4561
| _ -> ());
62+
63+
(*Gc.print_stat stdout;*)
4664
exit exitCode

‎bin/ODiffBin.ml

+8-1
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,13 @@ let diffLines =
5656
"With this flag enabled, output result in case of different images \
5757
will output lines for all the different pixels"
5858

59+
let disableGcOptimizations =
60+
value & flag
61+
& info [ "reduce-ram-usage" ]
62+
~doc:
63+
"With this flag enabled odiff will use less memory, but will be slower \
64+
in some cases."
65+
5966
let ignoreRegions =
6067
value
6168
& opt
@@ -76,7 +83,7 @@ let cmd =
7683
in
7784
( const Main.main $ base $ comp $ diffPath $ threshold $ diffMask
7885
$ failOnLayout $ diffColor $ parsableOutput $ antialiasing $ ignoreRegions
79-
$ diffLines,
86+
$ diffLines $ disableGcOptimizations,
8087
Term.info "odiff" ~version:"3.0.0" ~doc:"Find difference between 2 images."
8188
~exits:
8289
(Term.exit_info 0 ~doc:"on image match"

‎bin/node-bindings/odiff.d.ts

+2
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,8 @@ export type ODiffOptions = Partial<{
1313
antialiasing: boolean;
1414
/** If `true` reason: "pixel-diff" output will contain the set of line indexes containing different pixels */
1515
captureDiffLines: boolean;
16+
/** If `true` odiff will use less memory but will be slower with larger images */
17+
reduceRamUsage: boolean;
1618
/** An array of regions to ignore in the diff. */
1719
ignoreRegions: Array<{
1820
x1: number;

‎bin/node-bindings/odiff.js

+4
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,10 @@ function optionsToArgs(options) {
5050
setFlag("output-diff-lines", value);
5151
break;
5252

53+
case "reduceRamUsage":
54+
setFlag("reduce-ram-usage", value);
55+
break;
56+
5357
case "ignoreRegions": {
5458
const regions = value
5559
.map(

‎src/Diff.ml

+14-20
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,7 @@
1-
let redPixel = (255, 0, 0)
1+
(* Decimal representation of the RGBA in32 pixel red pixel *)
2+
let redPixel = Int32.of_int 4278190335
3+
4+
(* Decimal representation of the RGBA in32 pixel green pixel *)
25
let maxYIQPossibleDelta = 35215.
36

47
type 'a diffVariant = Layout | Pixel of ('a * int * float * int Stack.t)
@@ -18,18 +21,21 @@ module MakeDiff (IO1 : ImageIO.ImageIO) (IO2 : ImageIO.ImageIO) = struct
1821

1922
let compare (base : IO1.t ImageIO.img) (comp : IO2.t ImageIO.img)
2023
?(antialiasing = false) ?(outputDiffMask = false) ?(diffLines = false)
21-
?(diffPixel : int * int * int = redPixel) ?(threshold = 0.1)
22-
?(ignoreRegions = []) () =
24+
?diffPixel ?(threshold = 0.1) ?(ignoreRegions = []) () =
2325
let maxDelta = maxYIQPossibleDelta *. (threshold ** 2.) in
26+
let diffPixel = match diffPixel with Some x -> x | None -> redPixel in
2427
let diffOutput =
2528
match outputDiffMask with
2629
| true -> IO1.makeSameAsLayout base
2730
| false -> base
2831
in
29-
let diffPixelQueue = Queue.create () in
32+
33+
let diffCount = ref 0 in
3034
let diffLinesStack = Stack.create () in
3135
let countDifference x y =
32-
diffPixelQueue |> Queue.push (x, y);
36+
incr diffCount;
37+
IO1.setImgColor ~x ~y diffPixel diffOutput;
38+
3339
if
3440
diffLines
3541
&& (diffLinesStack |> Stack.is_empty || diffLinesStack |> Stack.top < y)
@@ -74,26 +80,14 @@ module MakeDiff (IO1 : ImageIO.ImageIO) (IO2 : ImageIO.ImageIO) = struct
7480
else incr x
7581
done;
7682

77-
let diffCount = diffPixelQueue |> Queue.length in
78-
(if diffCount > 0 then
79-
let r, g, b = diffPixel in
80-
let a = (255 land 255) lsl 24 in
81-
let b = (b land 255) lsl 16 in
82-
let g = (g land 255) lsl 8 in
83-
let r = (r land 255) lsl 0 in
84-
let diffPixel = Int32.of_int (a lor b lor g lor r) in
85-
diffPixelQueue
86-
|> Queue.iter (fun (x, y) ->
87-
diffOutput |> IO1.setImgColor ~x ~y diffPixel));
88-
8983
let diffPercentage =
90-
100.0 *. Float.of_int diffCount
84+
100.0 *. Float.of_int !diffCount
9185
/. (Float.of_int base.width *. Float.of_int base.height)
9286
in
93-
(diffOutput, diffCount, diffPercentage, diffLinesStack)
87+
(diffOutput, !diffCount, diffPercentage, diffLinesStack)
9488

9589
let diff (base : IO1.t ImageIO.img) (comp : IO2.t ImageIO.img) ~outputDiffMask
96-
?(threshold = 0.1) ?(diffPixel = redPixel) ?(failOnLayoutChange = true)
90+
?(threshold = 0.1) ~diffPixel ?(failOnLayoutChange = true)
9791
?(antialiasing = false) ?(diffLines = false) ?(ignoreRegions = []) () =
9892
if
9993
failOnLayoutChange = true

‎test/Test_Core.ml

+7-4
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,8 @@ let _ =
1313
PNG_Diff.compare img1 img2 ~outputDiffMask:false ~antialiasing:true
1414
()
1515
in
16-
(expect.int diffPixels).toBe 38;
17-
(expect.float diffPercentage).toBeCloseTo 0.095);
16+
(expect.int diffPixels).toBe 46;
17+
(expect.float diffPercentage).toBeCloseTo 0.115);
1818
test "tests different sized AA images" (fun { expect; _ } ->
1919
let img1 = loadImage "test/test-images/aa/antialiasing-on.png" in
2020
let img2 =
@@ -58,14 +58,17 @@ let _ =
5858

5959
let _ =
6060
describe "CORE: Diff Color" (fun { test; _ } ->
61-
test "creates diff output image with custom diff color"
61+
test "creates diff output image with custom green diff color"
6262
(fun { expect; _ } ->
6363
let img1 = Png.IO.loadImage "test/test-images/png/orange.png" in
6464
let img2 =
6565
Png.IO.loadImage "test/test-images/png/orange_changed.png"
6666
in
6767
let diffOutput, _, _, _ =
68-
PNG_Diff.compare img1 img2 ~diffPixel:(0, 255, 0) ()
68+
PNG_Diff.compare img1 img2
69+
~diffPixel:
70+
(Int32.of_int 4278255360 (*int32 representation of #00ff00*))
71+
()
6972
in
7073
let originalDiff =
7174
Png.IO.loadImage "test/test-images/png/orange_diff_green.png"

‎test/Test_IO_PNG.ml

+2-6
Original file line numberDiff line numberDiff line change
@@ -42,11 +42,7 @@ let _ =
4242

4343
test "Correctly handles different encodings of transparency"
4444
(fun { expect; _ } ->
45-
let img1 =
46-
loadImage "test/test-images/png/extreme-alpha.png"
47-
in
48-
let img2 =
49-
loadImage "test/test-images/png/extreme-alpha-1.png"
50-
in
45+
let img1 = loadImage "test/test-images/png/extreme-alpha.png" in
46+
let img2 = loadImage "test/test-images/png/extreme-alpha-1.png" in
5147
let _, diffPixels, _, _ = Diff.compare img1 img2 () in
5248
(expect.int diffPixels).toBe 0))

‎test/node-binding.test.cjs

+34-20
Original file line numberDiff line numberDiff line change
@@ -16,19 +16,37 @@ const BINARY_PATH = path.resolve(
1616

1717
console.log(`Testing binary ${BINARY_PATH}`);
1818

19+
const options = {
20+
__binaryPath: BINARY_PATH,
21+
}
22+
1923
test("Outputs correct parsed result when images different", async (t) => {
24+
const { reason, diffCount, diffPercentage } = await compare(
25+
path.join(IMAGES_PATH, "donkey.png"),
26+
path.join(IMAGES_PATH, "donkey-2.png"),
27+
path.join(IMAGES_PATH, "diff.png"),
28+
options
29+
);
30+
31+
t.is(reason, "pixel-diff");
32+
t.is(diffCount, 101841);
33+
t.is(diffPercentage, 2.65077570347);
34+
})
35+
36+
test("Correctly works with reduceRamUsage", async (t) => {
2037
const { reason, diffCount, diffPercentage } = await compare(
2138
path.join(IMAGES_PATH, "donkey.png"),
2239
path.join(IMAGES_PATH, "donkey-2.png"),
2340
path.join(IMAGES_PATH, "diff.png"),
2441
{
25-
__binaryPath: BINARY_PATH,
42+
...options,
43+
reduceRamUsage: true,
2644
}
2745
);
2846

2947
t.is(reason, "pixel-diff");
30-
t.is(diffCount, 109861);
31-
t.is(diffPercentage, 2.85952484323);
48+
t.is(diffCount, 101841);
49+
t.is(diffPercentage, 2.65077570347);
3250
});
3351

3452
test("Correctly parses threshold", async (t) => {
@@ -37,14 +55,14 @@ test("Correctly parses threshold", async (t) => {
3755
path.join(IMAGES_PATH, "donkey-2.png"),
3856
path.join(IMAGES_PATH, "diff.png"),
3957
{
40-
threshold: 0.6,
41-
__binaryPath: BINARY_PATH,
58+
...options,
59+
threshold: 0.5,
4260
}
4361
);
4462

4563
t.is(reason, "pixel-diff");
46-
t.is(diffCount, 50332);
47-
t.is(diffPercentage, 1.31007003768);
64+
t.is(diffCount, 65357);
65+
t.is(diffPercentage, 1.70114931758);
4866
});
4967

5068
test("Correctly parses antialiasing", async (t) => {
@@ -53,14 +71,14 @@ test("Correctly parses antialiasing", async (t) => {
5371
path.join(IMAGES_PATH, "donkey-2.png"),
5472
path.join(IMAGES_PATH, "diff.png"),
5573
{
74+
...options,
5675
antialiasing: true,
57-
__binaryPath: BINARY_PATH,
5876
}
5977
);
6078

6179
t.is(reason, "pixel-diff");
62-
t.is(diffCount, 108208);
63-
t.is(diffPercentage, 2.8164996153);
80+
t.is(diffCount, 101499);
81+
t.is(diffPercentage, 2.64187393218);
6482
});
6583

6684
test("Correctly parses ignore regions", async (t) => {
@@ -69,6 +87,7 @@ test("Correctly parses ignore regions", async (t) => {
6987
path.join(IMAGES_PATH, "donkey-2.png"),
7088
path.join(IMAGES_PATH, "diff.png"),
7189
{
90+
...options,
7291
ignoreRegions: [
7392
{
7493
x1: 749,
@@ -83,7 +102,6 @@ test("Correctly parses ignore regions", async (t) => {
83102
y2: 1334,
84103
},
85104
],
86-
__binaryPath: BINARY_PATH,
87105
}
88106
);
89107

@@ -95,9 +113,7 @@ test("Outputs correct parsed result when images different for cypress image", as
95113
path.join(IMAGES_PATH, "www.cypress.io.png"),
96114
path.join(IMAGES_PATH, "www.cypress.io-1.png"),
97115
path.join(IMAGES_PATH, "diff.png"),
98-
{
99-
__binaryPath: BINARY_PATH,
100-
}
116+
options
101117
);
102118

103119
t.is(reason, "pixel-diff");
@@ -110,9 +126,7 @@ test("Correctly handles same images", async (t) => {
110126
path.join(IMAGES_PATH, "donkey.png"),
111127
path.join(IMAGES_PATH, "donkey.png"),
112128
path.join(IMAGES_PATH, "diff.png"),
113-
{
114-
__binaryPath: BINARY_PATH,
115-
}
129+
options
116130
);
117131

118132
t.is(match, true);
@@ -125,12 +139,12 @@ test("Correctly outputs diff lines", async (t) => {
125139
path.join(IMAGES_PATH, "diff.png"),
126140
{
127141
captureDiffLines: true,
128-
__binaryPath: BINARY_PATH,
142+
...options
129143
}
130144
);
131145

132146
t.is(match, false);
133-
t.is(diffLines.length, 411);
147+
t.is(diffLines.length, 402);
134148
});
135149

136150
test("Returns meaningful error if file does not exist and noFailOnFsErrors", async (t) => {
@@ -139,8 +153,8 @@ test("Returns meaningful error if file does not exist and noFailOnFsErrors", asy
139153
path.join(IMAGES_PATH, "not-existing.png"),
140154
path.join(IMAGES_PATH, "diff.png"),
141155
{
156+
...options,
142157
noFailOnFsErrors: true,
143-
__binaryPath: BINARY_PATH,
144158
}
145159
);
146160

0 commit comments

Comments
 (0)