Skip to content

Commit

Permalink
refactor: use local loop variable in copyFromFrameBuffer
Browse files Browse the repository at this point in the history
This change allows the compiler to keep the loop variable (readPtr) in a
register and therefore avoid cache miss in what is essentially a more general
memcpy.

By analysing the assembly generated by both gcc 6.3.1, gcc 4.8.5 and clang 5.0
I found that these compilers interpret the mutable reference such that it
has to be written back into memory in every iteration.

The performance regression  was when upgrading the compilers for Foundry's
Nuke. Our original build of OpenEXR 2.2.0 built with GCC 4.1.2 did not exhibit
this behaviour. It yielded significant speed up in Nuke's writing speed.

Signed-off-by: Gyula Gubacsi <gyula.gubacsi@foundry.com>
  • Loading branch information
FnGyula committed Jul 14, 2020
1 parent ec03203 commit 2dc4965
Showing 1 changed file with 25 additions and 20 deletions.
45 changes: 25 additions & 20 deletions OpenEXR/IlmImf/ImfMisc.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1448,6 +1448,8 @@ copyFromFrameBuffer (char *& writePtr,
Compressor::Format format,
PixelType type)
{
char * localWritePtr = writePtr;
const char * localReadPtr = readPtr;
//
// Copy a horizontal row of pixels from a frame
// buffer to an output file's line or tile buffer.
Expand All @@ -1463,29 +1465,29 @@ copyFromFrameBuffer (char *& writePtr,
{
case OPENEXR_IMF_INTERNAL_NAMESPACE::UINT:

while (readPtr <= endPtr)
while (localReadPtr <= endPtr)
{
Xdr::write <CharPtrIO> (writePtr,
*(const unsigned int *) readPtr);
readPtr += xStride;
Xdr::write <CharPtrIO> (localWritePtr,
*(const unsigned int *) localReadPtr);
localReadPtr += xStride;
}
break;

case OPENEXR_IMF_INTERNAL_NAMESPACE::HALF:

while (readPtr <= endPtr)
while (localReadPtr <= endPtr)
{
Xdr::write <CharPtrIO> (writePtr, *(const half *) readPtr);
readPtr += xStride;
Xdr::write <CharPtrIO> (localWritePtr, *(const half *) localReadPtr);
localReadPtr += xStride;
}
break;

case OPENEXR_IMF_INTERNAL_NAMESPACE::FLOAT:

while (readPtr <= endPtr)
while (localReadPtr <= endPtr)
{
Xdr::write <CharPtrIO> (writePtr, *(const float *) readPtr);
readPtr += xStride;
Xdr::write <CharPtrIO> (localWritePtr, *(const float *) localReadPtr);
localReadPtr += xStride;
}
break;

Expand All @@ -1504,33 +1506,33 @@ copyFromFrameBuffer (char *& writePtr,
{
case OPENEXR_IMF_INTERNAL_NAMESPACE::UINT:

while (readPtr <= endPtr)
while (localReadPtr <= endPtr)
{
for (size_t i = 0; i < sizeof (unsigned int); ++i)
*writePtr++ = readPtr[i];
*localWritePtr++ = localReadPtr[i];

readPtr += xStride;
localReadPtr += xStride;
}
break;

case OPENEXR_IMF_INTERNAL_NAMESPACE::HALF:

while (readPtr <= endPtr)
while (localReadPtr <= endPtr)
{
*(half *) writePtr = *(const half *) readPtr;
writePtr += sizeof (half);
readPtr += xStride;
*(half *) localWritePtr = *(const half *) localReadPtr;
localWritePtr += sizeof (half);
localReadPtr += xStride;
}
break;

case OPENEXR_IMF_INTERNAL_NAMESPACE::FLOAT:

while (readPtr <= endPtr)
while (localReadPtr <= endPtr)
{
for (size_t i = 0; i < sizeof (float); ++i)
*writePtr++ = readPtr[i];
*localWritePtr++ = localReadPtr[i];

readPtr += xStride;
localReadPtr += xStride;
}
break;

Expand All @@ -1539,6 +1541,9 @@ copyFromFrameBuffer (char *& writePtr,
throw IEX_NAMESPACE::ArgExc ("Unknown pixel data type.");
}
}

writePtr = localWritePtr;
readPtr = localReadPtr;
}

void
Expand Down

0 comments on commit 2dc4965

Please sign in to comment.