Remove double-serialization for glGetProgramBinary

The applications get the binary length first, and then get the binary
itself.  Prior to this change, ANGLE was serializing the program binary
twice.

What's more, if the blob cache is enabled, ANGLE serialized the program
binary yet another time for that.

With this change, the program binary is serialized only once.  If the
application queries the program binary, serialization is done the first
time needed, is cached and then discarded as soon as the binary itself
is returned.

Bug: angleproject:7393
Change-Id: If6e3011097ca4d4a1cdcd2dcc23496901196d999
Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/5448090
Reviewed-by: Geoff Lang <geofflang@chromium.org>
Commit-Queue: Shahbaz Youssefi <syoussefi@chromium.org>

4 files changed, 51 insertions, 15 deletions

diff --git a/src/common/MemoryBuffer.h b/src/common/MemoryBuffer.h
index bcd3aab7a9..aeb96b94a2 100644
--- a/src/common/MemoryBuffer.h
+++ b/src/common/MemoryBuffer.h
@@ -59,10 +59,10 @@ class MemoryBuffer final : NonCopyable
 class ScratchBuffer final : NonCopyable
 {
   public:
+    ScratchBuffer();
     // If we request a scratch buffer requesting a smaller size this many times, release and
     // recreate the scratch buffer. This ensures we don't have a degenerate case where we are stuck
     // hogging memory.
-    ScratchBuffer();
     ScratchBuffer(uint32_t lifetime);
     ~ScratchBuffer();
 
diff --git a/src/libANGLE/MemoryProgramCache.cpp b/src/libANGLE/MemoryProgramCache.cpp
index a307a3e424..d93274b71e 100644
--- a/src/libANGLE/MemoryProgramCache.cpp
+++ b/src/libANGLE/MemoryProgramCache.cpp
@@ -182,8 +182,8 @@ angle::Result MemoryProgramCache::putProgram(const egl::BlobCache::Key &programH
         return angle::Result::Continue;
     }
 
-    angle::MemoryBuffer serializedProgram;
-    ANGLE_TRY(program->serialize(context, &serializedProgram));
+    ANGLE_TRY(program->serialize(context));
+    const angle::MemoryBuffer &serializedProgram = program->getSerializedBinary();
 
     angle::MemoryBuffer compressedData;
     if (!angle::CompressBlob(serializedProgram.size(), serializedProgram.data(), &compressedData))
diff --git a/src/libANGLE/Program.cpp b/src/libANGLE/Program.cpp
index 61435b2384..17d9182d13 100644
--- a/src/libANGLE/Program.cpp
+++ b/src/libANGLE/Program.cpp
@@ -768,8 +768,11 @@ void Program::onDestroy(const Context *context)
     ASSERT(!mState.hasAnyAttachedShader());
     SafeDelete(mProgram);
 
+    mBinary.clear();
+
     delete this;
 }
+
 ShaderProgramID Program::id() const
 {
     return mHandle;
@@ -887,6 +890,11 @@ void Program::makeNewExecutable(const Context *context)
 
     // If caching is disabled, consider it cached!
     mIsBinaryCached = context->getFrontendFeatures().disableProgramCaching.enabled;
+
+    // Start with a clean slate every time a new executable is installed.  Note that the executable
+    // binary is not mutable; once linked it remains constant.  When the program changes, a new
+    // executable is installed in this function.
+    mBinary.clear();
 }
 
 void Program::setupExecutableForLink(const Context *context)
@@ -927,8 +935,8 @@ void Program::setupExecutableForLink(const Context *context)
 
     // Make sure the executable state is in sync with the program.
     //
-    // The transform feedback buffer mode is duplicated in the executable as is the only link-input
-    // that is also needed at draw time.
+    // The transform feedback buffer mode is duplicated in the executable as it is the only
+    // link-input that is also needed at draw time.
     //
     // The transform feedback varying names are duplicated because the program pipeline link is not
     // currently able to use the link result of the program directly (and redoes the link, using
@@ -1452,11 +1460,14 @@ angle::Result Program::getBinary(Context *context,
         *binaryFormat = GL_PROGRAM_BINARY_ANGLE;
     }
 
-    angle::MemoryBuffer memoryBuf;
-    ANGLE_TRY(serialize(context, &memoryBuf));
+    // Serialize the program only if not already done.
+    if (mBinary.empty())
+    {
+        ANGLE_TRY(serialize(context));
+    }
 
-    GLsizei streamLength       = static_cast<GLsizei>(memoryBuf.size());
-    const uint8_t *streamState = memoryBuf.data();
+    GLsizei streamLength       = static_cast<GLsizei>(mBinary.size());
+    const uint8_t *streamState = mBinary.data();
 
     if (streamLength > bufSize)
     {
@@ -1479,6 +1490,12 @@ angle::Result Program::getBinary(Context *context,
         ptr += streamLength;
 
         ASSERT(ptr - streamLength == binary);
+
+        // Once the binary is retrieved, assume the application will never need the binary and
+        // release the memory.  Note that implicit caching to blob cache is disabled when the
+        // GL_PROGRAM_BINARY_RETRIEVABLE_HINT is set.  If that hint is not set, serialization is
+        // done twice, which is what the perf warning above is about!
+        mBinary.clear();
     }
 
     if (length)
@@ -2105,8 +2122,18 @@ bool Program::linkAttributes(const Caps &caps,
     return true;
 }
 
-angle::Result Program::serialize(const Context *context, angle::MemoryBuffer *binaryOut)
+angle::Result Program::serialize(const Context *context)
 {
+    // In typical applications, the binary should already be empty here.  However, in unusual
+    // situations this may not be true.  In particular, if the application doesn't set
+    // GL_PROGRAM_BINARY_RETRIEVABLE_HINT, gets the program length but doesn't get the binary, the
+    // cached binary remains until the program is destroyed or the program is bound (both causing
+    // |waitForPostLinkTasks()| to cache the program in the blob cache).
+    if (!mBinary.empty())
+    {
+        return angle::Result::Continue;
+    }
+
     BinaryOutputStream stream;
 
     stream.writeBytes(
@@ -2178,15 +2205,14 @@ angle::Result Program::serialize(const Context *context, angle::MemoryBuffer *bi
     mProgram->save(context, &stream);
     ASSERT(mState.mExecutable->mPostLinkSubTasks.empty());
 
-    ASSERT(binaryOut);
-    if (!binaryOut->resize(stream.length()))
+    if (!mBinary.resize(stream.length()))
     {
         ANGLE_PERF_WARNING(context->getState().getDebug(), GL_DEBUG_SEVERITY_LOW,
                            "Failed to allocate enough memory to serialize a program. (%zu bytes)",
                            stream.length());
         return angle::Result::Stop;
     }
-    memcpy(binaryOut->data(), stream.data(), stream.length());
+    memcpy(mBinary.data(), stream.data(), stream.length());
     return angle::Result::Continue;
 }
 
@@ -2337,6 +2363,10 @@ void Program::cacheProgramBinary(const Context *context)
             ANGLE_PERF_WARNING(context->getState().getDebug(), GL_DEBUG_SEVERITY_LOW,
                                "Failed to save linked program to memory program cache.");
         }
+
+        // Drop the binary; the application didn't specify that it wants to retrieve the binary.  If
+        // it did, we wouldn't be implicitly caching it.
+        mBinary.clear();
     }
 
     mIsBinaryCached = true;
diff --git a/src/libANGLE/Program.h b/src/libANGLE/Program.h
index 4bc8c75c26..50895ca265 100644
--- a/src/libANGLE/Program.h
+++ b/src/libANGLE/Program.h
@@ -466,8 +466,9 @@ class Program final : public LabeledObject, public angle::Subject
         }
     }
 
-    // Writes a program's binary to the output memory buffer.
-    angle::Result serialize(const Context *context, angle::MemoryBuffer *binaryOut);
+    // Writes a program's binary to |mBinary|.
+    angle::Result serialize(const Context *context);
+    const angle::MemoryBuffer &getSerializedBinary() const { return mBinary; }
 
     rx::UniqueSerial serial() const { return mSerial; }
 
@@ -561,6 +562,11 @@ class Program final : public LabeledObject, public angle::Subject
     // backends.
     ShaderMap<Shader *> mAttachedShaders;
 
+    // A cache of the program binary, prepared by |serialize()|.  OpenGL requires the application to
+    // query the length of the binary first (requiring a call to |serialize()|), and then get the
+    // actual binary.  This cache ensures the second call does not need to call |serialize()| again.
+    angle::MemoryBuffer mBinary;
+
     std::mutex mHistogramMutex;
 };
 }  // namespace gl