Fix caching of GL program binaries

Our "header" reading and writing code didn't agree, so we always failed
to recognize cached program binaries. The asserts in the testing sink
failed to notice, because we did get a 100% cache hit rate, but we
immediately discarded the data we received.

We now also check that we didn't insert anything into the cache, as a
proxy for doing any shader compile work. That change, plus the tweak to
set cached=false when the header fields are invalid (like we do if we
encounter problems further in the blob) detected the problem. Adding the
version tag to the start of the encoded blob fixes the test, and means
that program binary caching is actually working again.

This code still looks (and is) fragile. The next CL is going to rewrite
things to use SkReadBuffer and SkWriteBuffer, make the parsing code less
brittle, and give us a more robust way to detect failure anywhere in the
stream.

Bug: skia:9402
Change-Id: I0329f088e0afce3998494d91ef2206e5eb9cac42
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/294599
Reviewed-by: Brian Salomon <bsalomon@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
This commit is contained in:
Brian Osman 2020-06-05 11:11:36 -04:00 committed by Skia Commit-Bot
parent 288ecf60bd
commit 43f443f086
4 changed files with 16 additions and 3 deletions

View File

@ -1563,12 +1563,13 @@ Result GPUPersistentCacheTestingSink::draw(const Src& src, SkBitmap* dst, SkWStr
SkBitmap reference;
SkString refLog;
SkDynamicMemoryWStream refStream;
memoryCache.resetNumCacheMisses();
memoryCache.resetCacheStats();
Result refResult = this->onDraw(src, &reference, &refStream, &refLog, contextOptions);
if (!refResult.isOk()) {
return refResult;
}
SkASSERT(!memoryCache.numCacheMisses());
SkASSERT(!memoryCache.numCacheStores());
return compare_bitmaps(reference, *dst);
}

View File

@ -167,6 +167,7 @@ void GrGLProgramBuilder::storeShaderInCache(const SkSL::Program::Inputs& inputs,
GL_CALL(GetProgramiv(programID, GL_PROGRAM_BINARY_LENGTH, &length));
if (length > 0) {
SkWriter32 writer;
writer.write32(GrPersistentCacheUtils::kCurrentVersion);
writer.write32(kGLPB_Tag);
writer.writePad(&inputs, sizeof(inputs));
@ -300,6 +301,11 @@ sk_sp<GrGLProgram> GrGLProgramBuilder::finalize(const GrGLPrecompiledProgram* pr
sksl[i] = &cached_sksl[i];
}
break;
default:
// We got something invalid, so pretend it wasn't there
cached = false;
break;
}
}
if (!usedProgramBinaries) {

View File

@ -57,6 +57,7 @@ void MemoryCache::store(const SkData& key, const SkData& data) {
SkDebugf("Store Key: %s\n\tData: %s\n\n", data_to_str(key).c_str(),
data_to_str(data).c_str());
}
++fCacheStoreCnt;
fMap[Key(key)] = Value(data);
}

View File

@ -28,14 +28,18 @@ public:
MemoryCache(const MemoryCache&) = delete;
MemoryCache& operator=(const MemoryCache&) = delete;
void reset() {
fCacheMissCnt = 0;
this->resetCacheStats();
fMap.clear();
}
sk_sp<SkData> load(const SkData& key) override;
void store(const SkData& key, const SkData& data) override;
int numCacheMisses() const { return fCacheMissCnt; }
void resetNumCacheMisses() { fCacheMissCnt = 0; }
int numCacheStores() const { return fCacheStoreCnt; }
void resetCacheStats() {
fCacheMissCnt = 0;
fCacheStoreCnt = 0;
}
void writeShadersToDisk(const char* path, GrBackendApi backend);
@ -80,6 +84,7 @@ private:
};
int fCacheMissCnt = 0;
int fCacheStoreCnt = 0;
std::unordered_map<Key, Value, Hash> fMap;
};