From e79859ab1439e3200e26e38106281e2085e16fbe Mon Sep 17 00:00:00 2001 From: Hans Goudey Date: Fri, 31 Jan 2025 14:10:14 +0100 Subject: [PATCH] Refactor: Use Map instead of GHash for Python translations cache This makes the code simpler, aligns it with the builtin translations in messages.cc, and should improve performance as well. My motivation for the change is to allow returning StringRef/StringRefNull from the translation functions instead of char pointers. To test the change I used the Sun Position addon which includes translations and toggled different languages in the preferences. Pull Request: https://projects.blender.org/blender/blender/pulls/133827 --- .../blentranslation/BLT_translation.hh | 3 +- .../blentranslation/intern/blt_translation.cc | 6 +- .../python/intern/bpy_app_translations.cc | 114 +++++++++--------- 3 files changed, 64 insertions(+), 59 deletions(-) diff --git a/source/blender/blentranslation/BLT_translation.hh b/source/blender/blentranslation/BLT_translation.hh index 57c88ec234b..55dc27e694f 100644 --- a/source/blender/blentranslation/BLT_translation.hh +++ b/source/blender/blentranslation/BLT_translation.hh @@ -8,11 +8,12 @@ #pragma once +#include "BLI_string_ref.hh" #include "BLI_utildefines.h" /* for bool type */ #define TEXT_DOMAIN_NAME "blender" -bool BLT_is_default_context(const char *msgctxt); +bool BLT_is_default_context(blender::StringRef msgctxt); const char *BLT_pgettext(const char *msgctxt, const char *msgid); /* Translation */ diff --git a/source/blender/blentranslation/intern/blt_translation.cc b/source/blender/blentranslation/intern/blt_translation.cc index f5f85caad79..a259be32756 100644 --- a/source/blender/blentranslation/intern/blt_translation.cc +++ b/source/blender/blentranslation/intern/blt_translation.cc @@ -25,13 +25,15 @@ # include "messages.hh" #endif /* WITH_INTERNATIONAL */ -bool BLT_is_default_context(const char *msgctxt) +using blender::StringRef; + +bool BLT_is_default_context(const StringRef msgctxt) { /* We use the "short" test, a more complete one could be: * return (!msgctxt || !msgctxt[0] || STREQ(msgctxt, BLT_I18NCONTEXT_DEFAULT_BPYRNA)) */ /* NOTE: trying without the void string check for now, it *should* not be necessary... */ - return (!msgctxt || msgctxt[0] == BLT_I18NCONTEXT_DEFAULT_BPYRNA[0]); + return (msgctxt.is_empty() || msgctxt[0] == BLT_I18NCONTEXT_DEFAULT_BPYRNA[0]); } const char *BLT_pgettext(const char *msgctxt, const char *msgid) diff --git a/source/blender/python/intern/bpy_app_translations.cc b/source/blender/python/intern/bpy_app_translations.cc index e0aaa2ace75..d6b4d1ac262 100644 --- a/source/blender/python/intern/bpy_app_translations.cc +++ b/source/blender/python/intern/bpy_app_translations.cc @@ -27,8 +27,14 @@ #include "RNA_types.hh" #ifdef WITH_INTERNATIONAL -# include "BLI_ghash.h" + +# include "BLI_map.hh" # include "BLI_string.h" +# include "BLI_string_ref.hh" + +using blender::StringRef; +using blender::StringRefNull; + #endif /* ------------------------------------------------------------------- */ @@ -56,56 +62,47 @@ static BlenderAppTranslations *_translations = nullptr; /** \} */ /* ------------------------------------------------------------------- */ -/** \name Helpers for GHash +/** \name Helpers for hash * \{ */ #ifdef WITH_INTERNATIONAL -struct GHashKey { - const char *msgctxt; - const char *msgid; +struct MessageKeyRef { + StringRef context; + StringRef str; + + uint64_t hash() const + { + BLI_assert(this->context == BLT_I18NCONTEXT_DEFAULT_BPYRNA || + !BLT_is_default_context(this->context)); + return blender::get_default_hash(this->context, this->str); + } }; -static GHashKey *_ghashutil_keyalloc(const void *msgctxt, const void *msgid) -{ - GHashKey *key = static_cast(MEM_mallocN(sizeof(GHashKey), "Py i18n GHashKey")); - key->msgctxt = BLI_strdup(static_cast( - BLT_is_default_context(static_cast(msgctxt)) ? BLT_I18NCONTEXT_DEFAULT_BPYRNA : - msgctxt)); - key->msgid = BLI_strdup(static_cast(msgid)); - return key; -} +struct MessageKey { + std::string context; + std::string str; -static uint _ghashutil_keyhash(const void *ptr) -{ - const GHashKey *key = static_cast(ptr); - const uint hash = BLI_ghashutil_strhash(key->msgctxt); - return hash ^ BLI_ghashutil_strhash(key->msgid); -} - -static bool _ghashutil_keycmp(const void *a, const void *b) -{ - const GHashKey *A = static_cast(a); - const GHashKey *B = static_cast(b); - - /* NOTE: comparing msgid first, most of the time it will be enough! */ - if (BLI_ghashutil_strcmp(A->msgid, B->msgid) == false) { - return BLI_ghashutil_strcmp(A->msgctxt, B->msgctxt); + uint64_t hash() const + { + return blender::get_default_hash(this->context, this->str); } - return true; /* true means they are not equal! */ -} -static void _ghashutil_keyfree(void *ptr) + static uint64_t hash_as(const MessageKeyRef &key) + { + return key.hash(); + } +}; + +inline bool operator==(const MessageKey &a, const MessageKey &b) { - const GHashKey *key = static_cast(ptr); - - /* We assume both msgctxt and msgid were BLI_strdup'ed! */ - MEM_freeN((void *)key->msgctxt); - MEM_freeN((void *)key->msgid); - MEM_freeN((void *)key); + return a.context == b.context && a.str == b.str; } -# define _ghashutil_valfree MEM_freeN +inline bool operator==(const MessageKeyRef &a, const MessageKey &b) +{ + return a.context == b.context && a.str == b.str; +} /** \} */ @@ -115,19 +112,20 @@ static void _ghashutil_keyfree(void *ptr) /** * We cache all messages available for a given locale - * from all Python dictionaries into a single #GHash. + * from all Python dictionaries into a single #Map. * Changing of locale is not so common, while looking for a message translation is, - * so let's try to optimize the later as much as we can! + * so let's try to optimize the latter as much as we can! * Note changing of locale, as well as (un)registering a message dict, invalidate that cache. */ -static GHash *_translations_cache = nullptr; +static std::unique_ptr> &get_translations_cache() +{ + static std::unique_ptr> translations; + return translations; +} static void _clear_translations_cache() { - if (_translations_cache) { - BLI_ghash_free(_translations_cache, _ghashutil_keyfree, _ghashutil_valfree); - } - _translations_cache = nullptr; + get_translations_cache().reset(); } static void _build_translations_cache(PyObject *py_messages, const char *locale) @@ -143,7 +141,7 @@ static void _build_translations_cache(PyObject *py_messages, const char *locale) /* Clear the cached ghash if needed, and create a new one. */ _clear_translations_cache(); - _translations_cache = BLI_ghash_new(_ghashutil_keyhash, _ghashutil_keycmp, __func__); + get_translations_cache() = std::make_unique>(); /* Iterate over all Python dictionaries. */ while (PyDict_Next(py_messages, &pos, &uuid, &uuid_dict)) { @@ -239,10 +237,12 @@ static void _build_translations_cache(PyObject *py_messages, const char *locale) /* Do not overwrite existing keys! */ if (BPY_app_translations_py_pgettext(msgctxt, msgid) == msgid) { - GHashKey *key = _ghashutil_keyalloc(msgctxt, msgid); + MessageKey key; + key.context = BLT_is_default_context(msgctxt) ? BLT_I18NCONTEXT_DEFAULT_BPYRNA : msgctxt; + key.str = msgid; Py_ssize_t trans_str_len; const char *trans_str = PyUnicode_AsUTF8AndSize(trans, &trans_str_len); - BLI_ghash_insert(_translations_cache, key, BLI_strdupn(trans_str, trans_str_len)); + get_translations_cache()->add(key, std::string(trans_str, trans_str_len)); } } } @@ -258,7 +258,6 @@ const char *BPY_app_translations_py_pgettext(const char *msgctxt, const char *ms { # define STATIC_LOCALE_SIZE 32 /* Should be more than enough! */ - GHashKey key; static char locale[STATIC_LOCALE_SIZE] = ""; const char *tmp; @@ -268,7 +267,7 @@ const char *BPY_app_translations_py_pgettext(const char *msgctxt, const char *ms } tmp = BLT_lang_get(); - if (!STREQ(tmp, locale) || !_translations_cache) { + if (!STREQ(tmp, locale) || !get_translations_cache()) { PyGILState_STATE _py_state; STRNCPY(locale, tmp); @@ -283,12 +282,15 @@ const char *BPY_app_translations_py_pgettext(const char *msgctxt, const char *ms } /* And now, simply create the key (context, messageid) and find it in the cached dict! */ - key.msgctxt = BLT_is_default_context(msgctxt) ? BLT_I18NCONTEXT_DEFAULT_BPYRNA : msgctxt; - key.msgid = msgid; + MessageKeyRef key; + key.context = BLT_is_default_context(msgctxt) ? BLT_I18NCONTEXT_DEFAULT_BPYRNA : msgctxt; + key.str = msgid; - tmp = static_cast(BLI_ghash_lookup(_translations_cache, &key)); - - return tmp ? tmp : msgid; + const std::string *result = get_translations_cache()->lookup_ptr_as(key); + if (!result) { + return msgid; + } + return result->c_str(); # undef STATIC_LOCALE_SIZE }