Skip to content

Commit ea0291e

Browse files
csmartdaltoncsmartdalton
andcommitted
perf(tests): Spawn image_diff.py only once per job (#13013) c4ea178471
perf(goldens): Spawn image_diff.py only once per job diff.py was spawning a python process per image, and each re-imported cv2. This import absolutely DOMINATED the time spent diffing, especially on Windows. Instead, import cv2 once per job, and diff multiple images per image_diff.py process. Time improvement diffing gms on my laptop: 8.7 -> 2 seconds And diffing gms + goldens: 26 -> 10 seconds Co-authored-by: Chris Dalton <99840794+csmartdalton@users.noreply.github.com>
1 parent 56c7ef2 commit ea0291e

4 files changed

Lines changed: 186 additions & 109 deletions

File tree

.rive_head

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
19d8fb7957c4a37b699b530b5def3f355e2c164e
1+
c4ea1784715990dd1711c8cab3f79d6b59710581

tests/check_golds.sh

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -150,12 +150,12 @@ do
150150
python3 deploy_tests.py $TESTS $ARGS --target=$TARGET --outdir=.gold/$ID --backend=$BACKEND $NO_REBUILD
151151
else
152152
echo
153-
echo "Checking $ID..."
153+
echo "Deploying $ID..."
154154
rm -fr .gold/candidates/$ID
155155
python3 deploy_tests.py $TESTS $ARGS --target=$TARGET --outdir=.gold/candidates/$ID --backend=$BACKEND $NO_REBUILD
156156

157157
echo
158-
echo "Checking $ID..."
158+
echo "Diffing $ID..."
159159
rm -fr .gold/diffs/$ID && mkdir -p .gold/diffs/$ID
160160
python3 diff.py $DIFF_ARGS -g .gold/$ID -c .gold/candidates/$ID -j$NUMBER_OF_PROCESSORS -o .gold/diffs/$ID \
161161
|| open_file .gold/diffs/$ID/index.html

tests/diff.py

Lines changed: 52 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -29,8 +29,8 @@
2929
import argparse
3030
import glob
3131
import csv
32-
from multiprocessing import Pool
33-
from functools import partial
32+
import threading
33+
import queue
3434
from xml.etree import ElementTree as ET
3535
from typing import TypeVar
3636
import shutil
@@ -54,9 +54,6 @@
5454

5555
args = parser.parse_args()
5656

57-
# _winapi.WaitForMultipleObjects only supports 64 handles, which we exceed if we span >61 diff jobs.
58-
args.jobs = min(args.jobs, 61)
59-
6057
status_filename_base = "_imagediff_status"
6158
status_filename_pattern = f"{status_filename_base}_%i_*.txt" % os.getpid()
6259
show_commands = False
@@ -237,11 +234,6 @@ def shallow_copy_images(src, dest):
237234
for file in file_names:
238235
shutil.copyfile(file.path, os.path.join(dest, file.name))
239236

240-
def remove_suffix(name, oldsuffix):
241-
if name.endswith(oldsuffix):
242-
name = name[:-len(oldsuffix)]
243-
return name
244-
245237
def write_csv(entries, origpath, candidatepath, diffpath, missing_candidates):
246238
origpath = os.path.relpath(origpath, diffpath)
247239
candidatepath = os.path.relpath(candidatepath, diffpath)
@@ -301,29 +293,44 @@ def write_min_csv(total_passing, total_failing, total_missing_candidates, total_
301293
csv_writer.writerow({'type':'identical', 'number' : str(total_identical)})
302294
csv_writer.writerow({'type':'total', 'number' : str(total_entries)})
303295

304-
def call_imagediff(filename, golden, candidate, output, parent_pid):
305-
cmd = [PYTHON, "image_diff.py",
306-
"-n", remove_suffix(filename, ".png"),
307-
"-g", os.path.join(golden, filename),
308-
"-c", os.path.join(candidate, filename),
296+
def diff_worker(index, work_queue, golden, candidate, output, parent_pid):
297+
# One long-lived image_diff.py process that pulls filenames off the shared
298+
# work_queue one at a time. Keeping the process alive means cv2 is imported
299+
# only once (that import + process spawn dominate on Windows).
300+
cmd = [PYTHON, "image_diff.py", "--serve",
301+
"-g", golden,
302+
"-c", candidate,
309303
# Each process writes its own status file in order to avoid race conditions.
310-
"-s", "%s_%i_%i.txt" % (status_filename_base, parent_pid, os.getpid())]
304+
"-s", "%s_%i_%i.txt" % (status_filename_base, parent_pid, index)]
311305
if output is not None:
312306
cmd.extend(["-o", output])
313307
if args.verbose:
314308
cmd.extend(["-v", "-l"])
315309
if args.histogram_compare:
316310
cmd.extend(["-H"])
317-
311+
318312
if show_commands:
319-
str = ""
320-
for c in cmd:
321-
str += c + " "
322-
print(str)
323-
324-
if 0 != subprocess.call(cmd):
325-
print("Error calling " + cmd[0])
326-
return -1
313+
print(" ".join(cmd))
314+
315+
proc = subprocess.Popen(cmd, stdin=subprocess.PIPE, stdout=subprocess.PIPE,
316+
text=True)
317+
try:
318+
while True:
319+
try:
320+
filename = work_queue.get_nowait()
321+
except queue.Empty:
322+
break
323+
proc.stdin.write(filename + "\n")
324+
proc.stdin.flush()
325+
# Block until the worker signals it finished this image. If the pipe
326+
# closed early the worker died; stop feeding it (the missing status
327+
# line is caught by the line-count check in diff_directory_shallow).
328+
if proc.stdout.readline().strip() != "done":
329+
print("image_diff.py worker %i died processing %s" % (index, filename))
330+
break
331+
finally:
332+
proc.stdin.close()
333+
proc.wait()
327334

328335
def parse_status(candidates_path, golden_path, output_path, device_name, browserstack_details):
329336
total_lines = 0
@@ -364,14 +371,26 @@ def diff_directory_shallow(candidates_path, output_path, golden_path, device_nam
364371
print("Diffing %i candidates..." % len(intersect_filenames))
365372
sys.stdout.flush()
366373

367-
# generate the diffs (if any) and write to the status file
368-
f = partial(call_imagediff,
369-
golden=golden_path,
370-
candidate=candidates_path,
371-
output=output_path,
372-
parent_pid=os.getpid())
373-
374-
Pool(args.jobs).map(f, intersect_filenames)
374+
# Fill a shared queue with every filename.
375+
work_queue = queue.Queue()
376+
for filename in intersect_filenames:
377+
work_queue.put(filename)
378+
379+
# Generate the diffs (if any) and write to the status file(s). Start up to
380+
# `jobs` long-lived image_diff.py workers that each pull filenames off the
381+
# queue as fast as they can finish them.
382+
njobs = min(args.jobs, len(intersect_filenames))
383+
parent_pid = os.getpid()
384+
threads = []
385+
for index in range(njobs):
386+
t = threading.Thread(target=diff_worker,
387+
args=(index, work_queue, golden_path,
388+
candidates_path, output_path, parent_pid))
389+
t.start()
390+
threads.append(t)
391+
for t in threads:
392+
t.join()
393+
375394
(total_lines, entries, success) = parse_status(candidates_path, golden_path, output_path, device_name, browserstack_details)
376395

377396
entries.extend(missing)

tests/image_diff.py

Lines changed: 131 additions & 73 deletions
Original file line numberDiff line numberDiff line change
@@ -4,49 +4,75 @@
44
import cv2 as cv
55
import numpy as np
66
import os
7+
import sys
78

89
parser = argparse.ArgumentParser(description=
910
"""
10-
Compares to images by pixels, outputing two different images for the differences.
11+
Compares candidate images against goldens by pixels, writing one status line
12+
per image and (optionally) two diff images per image.
1113
12-
output file {status}
13-
output file {name}.diff0.png
14-
The exact abs(A - B) of each pixel
15-
output file {name}.diff1.png
16-
A mask which has 1 for difference and 0 for no difference at a given pixel
14+
A single process diffs multiple images in order to amortize the cost of spawning
15+
the Python interpreter and importing OpenCV (cv2).
16+
17+
Two ways to feed images to a process:
18+
--names diff a fixed list of file names, then exit.
19+
--serve stay alive reading one file name per line from stdin, writing
20+
"done" to stdout after each, so the parent (diff.py) can pull work
21+
dynamically and keep every worker busy until the queue is empty.
22+
23+
output file {status} one status line per image
24+
output file {name}.diff0.png the exact abs(A - B) of each pixel
25+
output file {name}.diff1.png a mask which has 255 for difference, 0 otherwise
1726
""")
1827

19-
parser.add_argument("-n", "--name", type=str, required=True, help="name used for output images")
20-
parser.add_argument("-c", "--candidate", type=str, required=True, help="candidate for image diffing")
21-
parser.add_argument("-g", "--golden", type=str, required=True, help="golden to compare against")
28+
mode = parser.add_mutually_exclusive_group(required=True)
29+
mode.add_argument("-N", "--names", type=str, nargs='+', help="space-separated image file names (e.g. foo.png bar.png) to diff; each is looked up by name in the --candidate and --golden directories")
30+
mode.add_argument("--serve", action='store_true', help="read image file names from stdin, one per line, diffing each and writing 'done' to stdout after each; exit on EOF")
31+
parser.add_argument("-c", "--candidate", type=str, required=True, help="candidate DIRECTORY of images to diff")
32+
parser.add_argument("-g", "--golden", type=str, required=True, help="golden DIRECTORY to compare against")
2233
parser.add_argument("-s", "--status", type=str, required=True, help="output stats file name and location")
2334
parser.add_argument("-o", "--outdir", type=str, help="output directory to store image diffs, if not provided then no output image is saved")
2435
parser.add_argument("-v", "--verbose", action='store_true', help="enable verbose logging")
25-
parser.add_argument("-l", "--log", action='store_true', help="redirect all verbose logging to a file with --name")
36+
parser.add_argument("-l", "--log", action='store_true', help="redirect all verbose logging to a per-image file")
2637
parser.add_argument("-H", "--histogram", action='store_true', help="compare images using histograms as an additional method for ruling out 'same' images. This should prevent subtle differences in the image that should not be visible from preventing a passing result")
2738

2839
args = parser.parse_args()
2940

30-
def verbose_log(val):
41+
def remove_suffix(name, oldsuffix):
42+
if name.endswith(oldsuffix):
43+
name = name[:-len(oldsuffix)]
44+
return name
45+
46+
def verbose_log(name, val):
3147
if not args.verbose:
3248
return
3349
if args.log:
34-
with open(f"tmp/{args.name}.log", "a") as log_file:
50+
with open(f"tmp/{name}.log", "a") as log_file:
3551
log_file.write(val+"\n")
3652
else:
37-
print(val)
38-
39-
def main():
40-
if args.log:
41-
os.makedirs("tmp", exist_ok=True)
42-
53+
print(val, file=sys.stderr)
54+
55+
def diff_one(filename, status):
56+
name = remove_suffix(filename, ".png")
57+
candidate_path = os.path.join(args.candidate, filename)
58+
golden_path = os.path.join(args.golden, filename)
59+
60+
candidate = None
61+
golden = None
62+
diff = None
63+
rgb_diff = None
64+
mask = None
65+
total_diff_count = 0
66+
max_diff = 0
67+
avg = 0.0
68+
hist_result = 1.0
4369
size_match = False
4470
failed = False
4571
try:
46-
verbose_log(f"loading {args.candidate}")
47-
candidate = cv.imread(args.candidate)
48-
verbose_log(f"loading {args.golden}")
49-
golden = cv.imread(args.golden)
72+
verbose_log(name, f"loading {candidate_path}")
73+
candidate = cv.imread(candidate_path)
74+
verbose_log(name, f"loading {golden_path}")
75+
golden = cv.imread(golden_path)
5076

5177
if candidate is not None and golden is not None:
5278
size_match = candidate.shape == golden.shape
@@ -85,67 +111,99 @@ def main():
85111
hist_candidate = cv.calcHist(hsv_candidate, [0,1], None, histSize, ranges, accumulate=False)
86112
hist_golden = cv.calcHist(hsv_golden, [0,1], None, histSize, ranges, accumulate=False)
87113

88-
# compare using CORREL histogram algorithm. the different options are detailed here
114+
# compare using CORREL histogram algorithm. the different options are detailed here
89115
# https://docs.opencv.org/3.4/d6/dc7/group__imgproc__hist.html#ga994f53817d621e2e4228fc646342d386
90116
# a hist_result of 1.0 means identical with this method. any variance results in a number less then 1.0
91117
hist_result = cv.compareHist(hist_candidate, hist_golden, cv.HISTCMP_CORREL)
92-
118+
93119
except Exception as E:
94-
print(f"Failed to load and process images {E}")
120+
print(f"Failed to load and process images {E}", file=sys.stderr)
95121
failed = True
96122

123+
status.write(name + "\t")
124+
if failed:
125+
status.write("failed\n")
126+
verbose_log(name, "failed to load golden or candidate")
127+
return
128+
if candidate is None:
129+
status.write("missing_candidate\n")
130+
verbose_log(name, "missing candidate for " + name)
131+
return
132+
if golden is None:
133+
status.write("missing_golden\n")
134+
verbose_log(name, "missing golden for " + name)
135+
return
136+
if failed:
137+
status.write("failed\n")
138+
verbose_log(name, "failed to load golden or candidate")
139+
return
140+
if total_diff_count == 0:
141+
status.write("identical\n")
142+
verbose_log(name, "files are identical")
143+
return
144+
if not size_match:
145+
status.write("sizemismatch\n")
146+
verbose_log(name, "files are not the same size")
147+
return
148+
status.write(str(max_diff)+"\t")
149+
# prevent python from writing out in scientific notation
150+
status.write(f"{float(avg):.5f}\t")
151+
status.write(str(total_diff_count)+"\t")
152+
status.write(str(diff.shape[0]*diff.shape[1]))
153+
# add our histogram result as the last value of the status file or write a new line to say we are finished with this status
154+
if args.histogram:
155+
status.write(f"\t{float(hist_result):.5f}\n")
156+
else:
157+
status.write("\n")
158+
159+
# save the output files if location provided
160+
if args.outdir:
161+
# color diff file location
162+
c_diff_path = os.path.join(args.outdir, name + ".diff0.png")
163+
# mask diff file location
164+
c_mask_path = os.path.join(args.outdir, name + ".diff1.png")
165+
verbose_log(name, f"writing images {c_diff_path} and {c_mask_path}")
166+
cv.imwrite(c_diff_path, rgb_diff)
167+
cv.imwrite(c_mask_path, mask)
168+
verbose_log(name, "finished writing output images")
169+
170+
def prepare_dirs():
171+
if args.log:
172+
os.makedirs("tmp", exist_ok=True)
173+
97174
# make path to stats file if necessary
98175
if os.path.dirname(args.status):
99-
verbose_log(f"making status file path {os.path.dirname(args.status)}")
100176
os.makedirs(os.path.dirname(args.status), exist_ok=True)
101-
verbose_log(f"making status file {args.status}")
102-
with open(args.status, "a") as status:
103-
status.write(args.name + "\t");
104-
if candidate is None:
105-
status.write("missing_candidate\n")
106-
verbose_log("missing golden for " + args.name)
107-
return
108-
if golden is None:
109-
status.write("missing_golden\n")
110-
verbose_log("missing golden for " + args.name)
111-
return
112-
if failed:
113-
status.write("failed\n")
114-
verbose_log("failed to load golden or candidate")
115-
return
116-
if total_diff_count == 0:
117-
status.write("identical\n")
118-
verbose_log("files are identical")
119-
return
120-
if not size_match:
121-
status.write("sizemismatch\n")
122-
verbose_log("files are not the same size")
123-
return
124-
status.write(str(max_diff)+"\t")
125-
# prevent python from writing out in scientific notation
126-
status.write(f"{float(avg):.5f}\t")
127-
status.write(str(total_diff_count)+"\t")
128-
status.write(str(diff.shape[0]*diff.shape[1]))
129-
# add our histogram result as the last value of the status file or write a new line to say we are finished with this status
130-
if args.histogram:
131-
status.write(f"\t{float(hist_result):.5f}\n")
132-
else:
133-
status.write("\n")
134-
135-
verbose_log("status file finished")
136-
# save the output file if location provided
137177
if args.outdir:
138-
# color diff file location
139-
c_diff_path = os.path.join(args.outdir, args.name + ".diff0.png")
140-
# mask diff file location
141-
c_mask_path = os.path.join(args.outdir, args.name + ".diff1.png")
142-
# create output dir if needed
143-
verbose_log(f"making output directory {args.outdir}")
144178
os.makedirs(args.outdir, exist_ok=True)
145-
verbose_log(f"writing images {c_diff_path} and {c_mask_path}")
146-
cv.imwrite(c_diff_path, rgb_diff)
147-
cv.imwrite(c_mask_path, mask)
148-
verbose_log("finished writing output images")
179+
180+
def main():
181+
# A single process handles the whole list; open the status file once and
182+
# append one line per image.
183+
prepare_dirs()
184+
with open(args.status, "a") as status:
185+
for filename in args.names:
186+
diff_one(filename, status)
187+
188+
def serve():
189+
# Interactive worker: the parent (diff.py) keeps this process alive and
190+
# feeds it one image file name per line on stdin, so cv2 is imported once
191+
# and then reused across many images. After each image we append its status
192+
# line to the status file and write "done" to stdout so the parent knows
193+
# we're ready for the next name. Exit on EOF.
194+
prepare_dirs()
195+
with open(args.status, "a") as status:
196+
for line in sys.stdin:
197+
filename = line.strip()
198+
if not filename:
199+
continue
200+
diff_one(filename, status)
201+
status.flush()
202+
sys.stdout.write("done\n")
203+
sys.stdout.flush()
149204

150205
if __name__ == '__main__':
151-
main()
206+
if args.serve:
207+
serve()
208+
else:
209+
main()

0 commit comments

Comments
 (0)