Skip to content

Commit 3b4cd1a

Browse files
cary-ilmcursoragent
cary-ilm
and
cursoragent
authored
Release Python GIL during I/O operations in bindings (AcademySoftwareFoundation#2273)
Release the GIL around OpenEXR read/write operations to allow other Python threads to run during file I/O. This prevents the bindings from blocking the entire Python interpreter during potentially lengthy operations. Operations wrapped with gil_scoped_release: - readPixels, readTiles, readPixelSampleCounts - writePixels, writeTiles, writeTile - copyPixels Also adds a test to verify the GIL is released during I/O. Signed-off-by: Cary Phillips <cary@ilm.com> Co-authored-by: Cursor <cursoragent@cursor.com>
1 parent 5667a27 commit 3b4cd1a

3 files changed

Lines changed: 123 additions & 16 deletions

File tree

‎src/wrappers/python/CMakeLists.txt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ if(BUILD_TESTING AND OPENEXR_TEST_PYTHON)
3636

3737
add_test(
3838
NAME OpenEXR.PyOpenEXR
39-
COMMAND ${Python3_EXECUTABLE} -m pytest ${CMAKE_CURRENT_SOURCE_DIR}/tests)
39+
COMMAND ${Python3_EXECUTABLE} -m pytest -v -s ${CMAKE_CURRENT_SOURCE_DIR}/tests)
4040

4141
set_tests_properties(OpenEXR.PyOpenEXR PROPERTIES
4242
ENVIRONMENT "PYTHONPATH=${CMAKE_CURRENT_BINARY_DIR}"

‎src/wrappers/python/PyOpenEXR.cpp‎

Lines changed: 54 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -340,7 +340,10 @@ PyPart::readPixels(MultiPartInputFile& infile, const ChannelList& channel_list,
340340
InputPart part (infile, part_index);
341341

342342
part.setFrameBuffer (frameBuffer);
343-
part.readPixels (dw.min.y, dw.max.y);
343+
{
344+
py::gil_scoped_release release;
345+
part.readPixels (dw.min.y, dw.max.y);
346+
}
344347
}
345348

346349
void
@@ -583,11 +586,17 @@ PyPart::readDeepPixels(MultiPartInputFile& infile, const std::string& type, cons
583586
{
584587
DeepScanLineInputPart part (infile, part_index);
585588
part.setFrameBuffer (frameBuffer);
586-
part.readPixelSampleCounts (dw.min.y, dw.max.y);
589+
{
590+
py::gil_scoped_release release;
591+
part.readPixelSampleCounts (dw.min.y, dw.max.y);
592+
}
587593

588594
setDeepSliceData(channel_list, height, width, sliceDataMap, rgbaChannelMap, sampleCount);
589595

590-
part.readPixels (dw.min.y, dw.max.y);
596+
{
597+
py::gil_scoped_release release;
598+
part.readPixels (dw.min.y, dw.max.y);
599+
}
591600
}
592601
else if (type == DEEPTILE)
593602
{
@@ -597,11 +606,17 @@ PyPart::readDeepPixels(MultiPartInputFile& infile, const std::string& type, cons
597606
int numXTiles = part.numXTiles (0);
598607
int numYTiles = part.numYTiles (0);
599608

600-
part.readPixelSampleCounts (0, numXTiles - 1, 0, numYTiles - 1);
609+
{
610+
py::gil_scoped_release release;
611+
part.readPixelSampleCounts (0, numXTiles - 1, 0, numYTiles - 1);
612+
}
601613

602614
setDeepSliceData(channel_list, height, width, sliceDataMap, rgbaChannelMap, sampleCount);
603615

604-
part.readTiles (0, numXTiles - 1, 0, numYTiles - 1);
616+
{
617+
py::gil_scoped_release release;
618+
part.readTiles (0, numXTiles - 1, 0, numYTiles - 1);
619+
}
605620
}
606621
}
607622

@@ -686,13 +701,19 @@ PyPart::writePixels(MultiPartOutputFile& outfile, const Box2i& dw) const
686701
{
687702
OutputPart part(outfile, part_index);
688703
part.setFrameBuffer (frameBuffer);
689-
part.writePixels (height());
704+
{
705+
py::gil_scoped_release release;
706+
part.writePixels (height());
707+
}
690708
}
691709
else
692710
{
693711
TiledOutputPart part(outfile, part_index);
694712
part.setFrameBuffer (frameBuffer);
695-
part.writeTiles (0, part.numXTiles() - 1, 0, part.numYTiles() - 1);
713+
{
714+
py::gil_scoped_release release;
715+
part.writeTiles (0, part.numXTiles() - 1, 0, part.numYTiles() - 1);
716+
}
696717
}
697718
}
698719

@@ -881,16 +902,22 @@ PyPart::writeDeepPixels(MultiPartOutputFile& outfile, const Box2i& dw) const
881902
{
882903
DeepScanLineOutputPart part(outfile, part_index);
883904
part.setFrameBuffer (frameBuffer);
884-
part.writePixels (height);
905+
{
906+
py::gil_scoped_release release;
907+
part.writePixels (height);
908+
}
885909
}
886910
else
887911
{
888912
DeepTiledOutputPart part(outfile, part_index);
889913
part.setFrameBuffer (frameBuffer);
890914

891-
for (int y = 0; y < part.numYTiles (0); y++)
892-
for (int x = 0; x < part.numXTiles (0); x++)
893-
part.writeTile (x, y, 0);
915+
{
916+
py::gil_scoped_release release;
917+
for (int y = 0; y < part.numYTiles (0); y++)
918+
for (int x = 0; x < part.numXTiles (0); x++)
919+
part.writeTile (x, y, 0);
920+
}
894921
}
895922
}
896923

@@ -1181,25 +1208,37 @@ PyFile::write(const char* outfilename)
11811208
{
11821209
InputPart inPart (*_inputFile, p);
11831210
OutputPart outPart (outfile, p);
1184-
outPart.copyPixels (inPart);
1211+
{
1212+
py::gil_scoped_release release;
1213+
outPart.copyPixels (inPart);
1214+
}
11851215
}
11861216
else if (type == TILEDIMAGE)
11871217
{
11881218
TiledInputPart inPart (*_inputFile, p);
11891219
TiledOutputPart outPart (outfile, p);
1190-
outPart.copyPixels (inPart);
1220+
{
1221+
py::gil_scoped_release release;
1222+
outPart.copyPixels (inPart);
1223+
}
11911224
}
11921225
else if (type == DEEPSCANLINE)
11931226
{
11941227
DeepScanLineInputPart inPart (*_inputFile, p);
11951228
DeepScanLineOutputPart outPart (outfile, p);
1196-
outPart.copyPixels (inPart);
1229+
{
1230+
py::gil_scoped_release release;
1231+
outPart.copyPixels (inPart);
1232+
}
11971233
}
11981234
else if (type == DEEPTILE)
11991235
{
12001236
DeepTiledInputPart inPart (*_inputFile, p);
12011237
DeepTiledOutputPart outPart (outfile, p);
1202-
outPart.copyPixels (inPart);
1238+
{
1239+
py::gil_scoped_release release;
1240+
outPart.copyPixels (inPart);
1241+
}
12031242
}
12041243
}
12051244
}

‎src/wrappers/python/tests/test_unittest.py‎

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -662,5 +662,73 @@ def make_header():
662662
with OpenEXR.File(outfilename, separate_channels=True) as i:
663663
compare_files (i, outfile2)
664664

665+
def test_gil_released_during_io(self):
666+
667+
#
668+
# Verify that the GIL is released during I/O operations by checking
669+
# that a background thread can make progress while OpenEXR performs
670+
# file operations.
671+
#
672+
673+
import threading
674+
import time
675+
676+
counter = [0]
677+
stop_flag = [False]
678+
679+
def background_worker():
680+
while not stop_flag[0]:
681+
counter[0] += 1
682+
time.sleep(0.001)
683+
684+
# Create a moderately large image to ensure I/O takes measurable time
685+
width = 1000
686+
height = 1000
687+
size = width * height
688+
R = np.random.rand(height, width).astype('f')
689+
G = np.random.rand(height, width).astype('f')
690+
B = np.random.rand(height, width).astype('f')
691+
channels = {
692+
"R": OpenEXR.Channel("R", R),
693+
"G": OpenEXR.Channel("G", G),
694+
"B": OpenEXR.Channel("B", B),
695+
}
696+
697+
# Start background thread
698+
thread = threading.Thread(target=background_worker)
699+
thread.start()
700+
701+
# Let thread run briefly to establish baseline
702+
time.sleep(0.05)
703+
count_before = counter[0]
704+
705+
# Perform I/O - if GIL is released, background thread will increment counter
706+
outfilename = mktemp_outfilename()
707+
with OpenEXR.File({}, channels) as outfile:
708+
outfile.write(outfilename)
709+
710+
count_after_write = counter[0]
711+
712+
# Now test reading
713+
with OpenEXR.File(outfilename) as infile:
714+
_ = infile.channels()
715+
716+
count_after_read = counter[0]
717+
718+
# Stop background thread
719+
stop_flag[0] = True
720+
thread.join()
721+
722+
# If GIL was released, background thread should have made progress during I/O
723+
write_progress = count_after_write - count_before
724+
read_progress = count_after_read - count_after_write
725+
726+
self.assertGreater(write_progress, 0,
727+
f"Background thread made no progress during write (GIL not released?)")
728+
self.assertGreater(read_progress, 0,
729+
f"Background thread made no progress during read (GIL not released?)")
730+
731+
os.remove(outfilename)
732+
665733
if __name__ == '__main__':
666734
unittest.main()

0 commit comments

Comments
 (0)