From 1e4bf308da820472c31ac5c387510f265a7c056e Mon Sep 17 00:00:00 2001 From: Hermet Park Date: Tue, 5 Mar 2024 17:15:49 +0900 Subject: [PATCH] renderer/loader: revamping the caching mechanism. The previous loader cache mechanism encountered a problem when the user changed the content of the cached data. In such cases, a new request would not be processed because the renderer would use the previously cached content. So far, the TVG cache mechanism utilizes a pointer hash key for the fastest hashing mechanism available. One limitation is that it assumes the address is unique for the data. To resolve this, we modified the caching policy. Now, the renderer will not cache copied data; it will only cache the given data when it is deemed shareable. issue: https://github.com/thorvg/thorvg/issues/2020 --- inc/thorvg.h | 16 ++++++++++++ src/bindings/capi/thorvg_capi.h | 12 +++++++++ src/renderer/tvgLoadModule.h | 15 ++++++++--- src/renderer/tvgLoader.cpp | 44 +++++++++++++++++++++------------ 4 files changed, 68 insertions(+), 19 deletions(-) diff --git a/inc/thorvg.h b/inc/thorvg.h index 5f2df08b..e571507b 100644 --- a/inc/thorvg.h +++ b/inc/thorvg.h @@ -1203,6 +1203,10 @@ public: /** * @brief Loads a picture data directly from a file. * + * ThorVG efficiently caches the loaded data using the specified @p path as a key. + * This means that loading the same file again will not result in duplicate operations; + * instead, ThorVG will reuse the previously loaded picture data. + * * @param[in] path A path to the picture file. * * @retval Result::Success When succeed. @@ -1218,6 +1222,10 @@ public: /** * @brief Loads a picture data from a memory block of a given size. * + * ThorVG efficiently caches the loaded data using the specified @p data address as a key + * when the @p copy has @c false. This means that loading the same data again will not result in duplicate operations + * for the sharable @p data. Instead, ThorVG will reuse the previously loaded picture data. + * * @param[in] data A pointer to a memory location where the content of the picture file is stored. * @param[in] size The size in bytes of the memory occupied by the @p data. * @param[in] mimeType Mimetype or extension of data such as "jpg", "jpeg", "lottie", "svg", "svg+xml", "png", etc. In case an empty string or an unknown type is provided, the loaders will be tried one by one. @@ -1262,6 +1270,10 @@ public: /** * @brief Loads a raw data from a memory block with a given size. * + * ThorVG efficiently caches the loaded data using the specified @p data address as a key + * when the @p copy has @c false. This means that loading the same data again will not result in duplicate operations + * for the sharable @p data. Instead, ThorVG will reuse the previously loaded picture data. + * * @param[in] paint A Tvg_Paint pointer to the picture object. * @param[in] data A pointer to a memory location where the content of the picture raw data is stored. * @param[in] w The width of the image @p data in pixels. @@ -1495,6 +1507,10 @@ public: /** * @brief Loads a scalable font data(ttf) from a file. * + * ThorVG efficiently caches the loaded data using the specified @p path as a key. + * This means that loading the same file again will not result in duplicate operations; + * instead, ThorVG will reuse the previously loaded font data. + * * @param[in] path The path to the font file. * * @retval Result::Success When succeed. diff --git a/src/bindings/capi/thorvg_capi.h b/src/bindings/capi/thorvg_capi.h index 6de03276..592d4b63 100644 --- a/src/bindings/capi/thorvg_capi.h +++ b/src/bindings/capi/thorvg_capi.h @@ -1952,6 +1952,10 @@ TVG_API Tvg_Paint* tvg_picture_new(); /*! * \brief Loads a picture data directly from a file. * +* ThorVG efficiently caches the loaded data using the specified @p path as a key. +* This means that loading the same file again will not result in duplicate operations; +* instead, ThorVG will reuse the previously loaded picture data. +* * \param[in] paint A Tvg_Paint pointer to the picture object. * \param[in] path The absolute path to the image file. * @@ -1967,6 +1971,10 @@ TVG_API Tvg_Result tvg_picture_load(Tvg_Paint* paint, const char* path); /*! * \brief Loads a picture data from a memory block of a given size. * +* ThorVG efficiently caches the loaded data using the specified @p data address as a key +* when the @p copy has @c false. This means that loading the same data again will not result in duplicate operations +* for the sharable @p data. Instead, ThorVG will reuse the previously loaded picture data. +* * \param[in] paint A Tvg_Paint pointer to the picture object. * \param[in] data A pointer to a memory location where the content of the picture raw data is stored. * \param[in] w The width of the image @p data in pixels. @@ -1988,6 +1996,10 @@ TVG_API Tvg_Result tvg_picture_load_raw(Tvg_Paint* paint, uint32_t *data, uint32 /*! * \brief Loads a picture data from a memory block of a given size. * +* ThorVG efficiently caches the loaded data using the specified @p data address as a key +* when the @p copy has @c false. This means that loading the same data again will not result in duplicate operations +* for the sharable @p data. Instead, ThorVG will reuse the previously loaded picture data. +* * \param[in] paint A Tvg_Paint pointer to the picture object. * \param[in] data A pointer to a memory location where the content of the picture file is stored. * \param[in] size The size in bytes of the memory occupied by the @p data. diff --git a/src/renderer/tvgLoadModule.h b/src/renderer/tvgLoadModule.h index 1f7ea63e..84d35bc0 100644 --- a/src/renderer/tvgLoadModule.h +++ b/src/renderer/tvgLoadModule.h @@ -32,17 +32,20 @@ struct LoadModule INLIST_ITEM(LoadModule); //Use either hashkey(data) or hashpath(path) - uint64_t hashkey; - char* hashpath = nullptr; + union { + uintptr_t hashkey; + char* hashpath = nullptr; + }; FileType type; //current loader file type uint16_t sharing = 0; //reference count bool readied = false; //read done already. + bool pathcache = false; //cached by path LoadModule(FileType type) : type(type) {} virtual ~LoadModule() { - free(hashpath); + if (pathcache) free(hashpath); } virtual bool open(const string& path) { return false; } @@ -57,6 +60,12 @@ struct LoadModule return true; } + bool cached() + { + if (hashkey) return true; + return false; + } + virtual bool close() { if (sharing == 0) return true; diff --git a/src/renderer/tvgLoader.cpp b/src/renderer/tvgLoader.cpp index bde4bbd3..661c5070 100644 --- a/src/renderer/tvgLoader.cpp +++ b/src/renderer/tvgLoader.cpp @@ -57,9 +57,9 @@ #include "tvgRawLoader.h" -uint64_t HASH_KEY(const char* data, uint64_t size) +uintptr_t HASH_KEY(const char* data) { - return (((uint64_t) data) << 32) | size; + return reinterpret_cast(data); } /************************************************************************/ @@ -219,7 +219,7 @@ static LoadModule* _findFromCache(const string& path) auto loader = _activeLoaders.head; while (loader) { - if (loader->hashpath && !strcmp(loader->hashpath, path.c_str())) { + if (loader->pathcache && !strcmp(loader->hashpath, path.c_str())) { ++loader->sharing; return loader; } @@ -237,7 +237,7 @@ static LoadModule* _findFromCache(const char* data, uint32_t size, const string& ScopedLock lock(key); auto loader = _activeLoaders.head; - auto key = HASH_KEY(data, size); + auto key = HASH_KEY(data); while (loader) { if (loader->type == type && loader->hashkey == key) { @@ -281,7 +281,7 @@ bool LoaderMgr::retrieve(LoadModule* loader) { if (!loader) return false; if (loader->close()) { - { + if (loader->cached()) { ScopedLock lock(key); _activeLoaders.remove(loader); } @@ -300,6 +300,7 @@ LoadModule* LoaderMgr::loader(const string& path, bool* invalid) if (auto loader = _findByPath(path)) { if (loader->open(path)) { loader->hashpath = strdup(path.c_str()); + loader->pathcache = true; { ScopedLock lock(key); _activeLoaders.back(loader); @@ -313,6 +314,7 @@ LoadModule* LoaderMgr::loader(const string& path, bool* invalid) if (auto loader = _find(static_cast(i))) { if (loader->open(path)) { loader->hashpath = strdup(path.c_str()); + loader->pathcache = true; { ScopedLock lock(key); _activeLoaders.back(loader); @@ -338,7 +340,7 @@ LoadModule* LoaderMgr::loader(const char* key) auto loader = _activeLoaders.head; while (loader) { - if (loader->hashpath && strstr(loader->hashpath, key)) { + if (loader->pathcache && strstr(loader->hashpath, key)) { ++loader->sharing; return loader; } @@ -350,15 +352,21 @@ LoadModule* LoaderMgr::loader(const char* key) LoadModule* LoaderMgr::loader(const char* data, uint32_t size, const string& mimeType, const string& rpath, bool copy) { - if (auto loader = _findFromCache(data, size, mimeType)) return loader; + //Note that users could use the same data pointer with the different content. + //Thus caching is only valid for shareable. + if (!copy) { + if (auto loader = _findFromCache(data, size, mimeType)) return loader; + } //Try with the given MimeType if (!mimeType.empty()) { if (auto loader = _findByType(mimeType)) { if (loader->open(data, size, rpath, copy)) { - loader->hashkey = HASH_KEY(data, size); - ScopedLock lock(key); - _activeLoaders.back(loader); + if (!copy) { + loader->hashkey = HASH_KEY(data); + ScopedLock lock(key); + _activeLoaders.back(loader); + } return loader; } else { TVGLOG("LOADER", "Given mimetype \"%s\" seems incorrect or not supported.", mimeType.c_str()); @@ -371,8 +379,8 @@ LoadModule* LoaderMgr::loader(const char* data, uint32_t size, const string& mim auto loader = _find(static_cast(i)); if (loader) { if (loader->open(data, size, rpath, copy)) { - loader->hashkey = HASH_KEY(data, size); - { + if (!copy) { + loader->hashkey = HASH_KEY(data); ScopedLock lock(key); _activeLoaders.back(loader); } @@ -387,14 +395,18 @@ LoadModule* LoaderMgr::loader(const char* data, uint32_t size, const string& mim LoadModule* LoaderMgr::loader(const uint32_t *data, uint32_t w, uint32_t h, bool premultiplied, bool copy) { - //TODO: should we check premultiplied?? - if (auto loader = _findFromCache((const char*)(data), w * h, "raw")) return loader; + //Note that users could use the same data pointer with the different content. + //Thus caching is only valid for shareable. + if (!copy) { + //TODO: should we check premultiplied?? + if (auto loader = _findFromCache((const char*)(data), w * h, "raw")) return loader; + } //function is dedicated for raw images only auto loader = new RawLoader; if (loader->open(data, w, h, premultiplied, copy)) { - loader->hashkey = HASH_KEY((const char*)data, w * h); - { + if (!copy) { + loader->hashkey = HASH_KEY((const char*)data); ScopedLock lock(key); _activeLoaders.back(loader); }