Cache locale string lookups instead of re-scanning the PO file every call
assetLocaleGetString rewinds and linearly scans/re-decompresses the whole locale file from byte 0 on every single call, with no caching - a text-heavy screen can easily make 10+ of these in a row (e.g. opening the game menu), and on PSP the containing archive is already fully resident in RAM, so the repeated cost is pure CPU (decompression + scanning), not I/O. Adds a fixed 128-entry move-to-front LRU cache keyed by (messageId, pluralCount), capped at 64/256 bytes per key/value (~40KB total) so the cost stays bounded no matter how large the game's script ends up being, rather than caching the whole locale file's text. The cache is a lazily-allocated pointer on assetlocalefile_t, not embedded inline - that struct lives inside the assetloaderoutput_t union shared by every asset type, and all ASSET_ENTRY_COUNT_MAX asset slots carry that union directly, so embedding it would have sized every slot up by ~40KB regardless of what asset type occupies it. Also fixes a bug this surfaced in test_assetlocale.c's own fixture: locale_teardown zeroed the locale struct directly instead of going through assetLocaleDispose, which would have leaked the new cache allocation across tests. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
@@ -9,6 +9,7 @@
|
||||
#include "asset/loader/locale/assetlocaleloader.h"
|
||||
#include "asset/asset.h"
|
||||
#include "util/memory.h"
|
||||
#include "util/string.h"
|
||||
#include <zip.h>
|
||||
|
||||
// ============================================================
|
||||
@@ -128,6 +129,7 @@ static int locale_teardown(void **state) {
|
||||
}
|
||||
|
||||
ASSET.zip = NULL;
|
||||
if(g_locale.cache != NULL) memoryFree(g_locale.cache);
|
||||
memoryZero(&g_locale, sizeof(g_locale));
|
||||
return 0;
|
||||
}
|
||||
@@ -311,7 +313,10 @@ static void test_getString_simple(void **state) {
|
||||
|
||||
assert_true(errorIsOk(ret));
|
||||
assert_string_equal(result, "Hello, World!");
|
||||
assert_int_equal(memoryGetAllocatedCount(), 0);
|
||||
// 1, not 0: the locale-string cache is lazily allocated on first use (the
|
||||
// header lookup in locale_setup already triggered it) and lives for the
|
||||
// rest of this file's lifetime - it's a bounded one-time cost, not a leak.
|
||||
assert_int_equal(memoryGetAllocatedCount(), 1);
|
||||
}
|
||||
|
||||
static void test_getString_plural_singular(void **state) {
|
||||
@@ -320,7 +325,7 @@ static void test_getString_plural_singular(void **state) {
|
||||
|
||||
assert_true(errorIsOk(ret));
|
||||
assert_string_equal(result, "one item");
|
||||
assert_int_equal(memoryGetAllocatedCount(), 0);
|
||||
assert_int_equal(memoryGetAllocatedCount(), 1);// see test_getString_simple
|
||||
}
|
||||
|
||||
static void test_getString_plural_many(void **state) {
|
||||
@@ -329,7 +334,7 @@ static void test_getString_plural_many(void **state) {
|
||||
|
||||
assert_true(errorIsOk(ret));
|
||||
assert_string_equal(result, "many items");
|
||||
assert_int_equal(memoryGetAllocatedCount(), 0);
|
||||
assert_int_equal(memoryGetAllocatedCount(), 1);// see test_getString_simple
|
||||
}
|
||||
|
||||
static void test_getString_multiple_calls(void **state) {
|
||||
@@ -337,12 +342,13 @@ static void test_getString_multiple_calls(void **state) {
|
||||
errorret_t ret = assetLocaleGetString(&g_locale, "greeting", 0, a, sizeof(a));
|
||||
assert_true(errorIsOk(ret));
|
||||
|
||||
// Second call rewinds the file and re-reads from scratch.
|
||||
// Second call is now served straight from the cache the first call
|
||||
// populated, rather than rewinding the file and re-scanning from scratch.
|
||||
ret = assetLocaleGetString(&g_locale, "greeting", 0, b, sizeof(b));
|
||||
assert_true(errorIsOk(ret));
|
||||
|
||||
assert_string_equal(a, b);
|
||||
assert_int_equal(memoryGetAllocatedCount(), 0);
|
||||
assert_int_equal(memoryGetAllocatedCount(), 1);// see test_getString_simple
|
||||
}
|
||||
|
||||
static void test_getString_missing_id(void **state) {
|
||||
@@ -351,9 +357,272 @@ static void test_getString_missing_id(void **state) {
|
||||
|
||||
assert_true(errorIsNotOk(ret));
|
||||
errorCatch(ret);
|
||||
assert_int_equal(memoryGetAllocatedCount(), 1);// see test_getString_simple
|
||||
}
|
||||
|
||||
// ============================================================
|
||||
// assetLocaleCacheFind / assetLocaleCacheInsert - pure tests
|
||||
// ============================================================
|
||||
// These call the cache directly rather than through assetLocaleGetString -
|
||||
// no open file is ever needed since the cache never touches file->file.
|
||||
|
||||
static void test_cache_findMiss_onEmptyCache(void **state) {
|
||||
assetlocalefile_t locale;
|
||||
memoryZero(&locale, sizeof(locale));
|
||||
|
||||
char_t result[64];
|
||||
bool_t hit = true;
|
||||
errorret_t ret = assetLocaleCacheFind(&locale, "greeting", 0, result, sizeof(result), &hit);
|
||||
|
||||
assert_true(errorIsOk(ret));
|
||||
assert_false(hit);
|
||||
assert_null(locale.cache);// a pure miss must not allocate anything
|
||||
assert_int_equal(memoryGetAllocatedCount(), 0);
|
||||
}
|
||||
|
||||
static void test_cache_insertThenFind_hits(void **state) {
|
||||
assetlocalefile_t locale;
|
||||
memoryZero(&locale, sizeof(locale));
|
||||
|
||||
assetLocaleCacheInsert(&locale, "greeting", 0, "Hello, World!");
|
||||
assert_non_null(locale.cache);
|
||||
|
||||
char_t result[64];
|
||||
bool_t hit = false;
|
||||
errorret_t ret = assetLocaleCacheFind(&locale, "greeting", 0, result, sizeof(result), &hit);
|
||||
|
||||
assert_true(errorIsOk(ret));
|
||||
assert_true(hit);
|
||||
assert_string_equal(result, "Hello, World!");
|
||||
|
||||
memoryFree(locale.cache);
|
||||
assert_int_equal(memoryGetAllocatedCount(), 0);
|
||||
}
|
||||
|
||||
static void test_cache_distinguishesByPluralCount(void **state) {
|
||||
assetlocalefile_t locale;
|
||||
memoryZero(&locale, sizeof(locale));
|
||||
|
||||
assetLocaleCacheInsert(&locale, "item", 1, "one item");
|
||||
assetLocaleCacheInsert(&locale, "item", 5, "many items");
|
||||
|
||||
char_t result[64];
|
||||
bool_t hit = false;
|
||||
|
||||
errorret_t ret = assetLocaleCacheFind(&locale, "item", 1, result, sizeof(result), &hit);
|
||||
assert_true(errorIsOk(ret));
|
||||
assert_true(hit);
|
||||
assert_string_equal(result, "one item");
|
||||
|
||||
ret = assetLocaleCacheFind(&locale, "item", 5, result, sizeof(result), &hit);
|
||||
assert_true(errorIsOk(ret));
|
||||
assert_true(hit);
|
||||
assert_string_equal(result, "many items");
|
||||
|
||||
memoryFree(locale.cache);
|
||||
}
|
||||
|
||||
static void test_cache_find_missOnDifferentMessageId(void **state) {
|
||||
assetlocalefile_t locale;
|
||||
memoryZero(&locale, sizeof(locale));
|
||||
|
||||
assetLocaleCacheInsert(&locale, "greeting", 0, "Hello, World!");
|
||||
|
||||
char_t result[64];
|
||||
bool_t hit = true;
|
||||
errorret_t ret = assetLocaleCacheFind(&locale, "farewell", 0, result, sizeof(result), &hit);
|
||||
|
||||
assert_true(errorIsOk(ret));
|
||||
assert_false(hit);
|
||||
|
||||
memoryFree(locale.cache);
|
||||
}
|
||||
|
||||
static void test_cache_find_bufferTooSmall_errors(void **state) {
|
||||
assetlocalefile_t locale;
|
||||
memoryZero(&locale, sizeof(locale));
|
||||
|
||||
assetLocaleCacheInsert(&locale, "greeting", 0, "Hello, World!");
|
||||
|
||||
char_t result[4];// too small for "Hello, World!"
|
||||
bool_t hit = false;
|
||||
errorret_t ret = assetLocaleCacheFind(&locale, "greeting", 0, result, sizeof(result), &hit);
|
||||
|
||||
assert_true(errorIsNotOk(ret));
|
||||
errorCatch(ret);
|
||||
|
||||
memoryFree(locale.cache);
|
||||
}
|
||||
|
||||
static void test_cache_insert_tooLongMessageId_notCached(void **state) {
|
||||
assetlocalefile_t locale;
|
||||
memoryZero(&locale, sizeof(locale));
|
||||
|
||||
char_t longId[ASSET_LOCALE_CACHE_KEY_MAX + 1];
|
||||
memorySet(longId, 'a', sizeof(longId) - 1);
|
||||
longId[sizeof(longId) - 1] = '\0';
|
||||
|
||||
assetLocaleCacheInsert(&locale, longId, 0, "value");
|
||||
|
||||
// Nothing should have been cached - the cache itself is never even
|
||||
// allocated, since this was the only insert attempted.
|
||||
assert_null(locale.cache);
|
||||
assert_int_equal(memoryGetAllocatedCount(), 0);
|
||||
}
|
||||
|
||||
static void test_cache_insert_tooLongValue_notCached(void **state) {
|
||||
assetlocalefile_t locale;
|
||||
memoryZero(&locale, sizeof(locale));
|
||||
|
||||
char_t longValue[ASSET_LOCALE_CACHE_VALUE_MAX + 1];
|
||||
memorySet(longValue, 'a', sizeof(longValue) - 1);
|
||||
longValue[sizeof(longValue) - 1] = '\0';
|
||||
|
||||
assetLocaleCacheInsert(&locale, "greeting", 0, longValue);
|
||||
|
||||
assert_null(locale.cache);
|
||||
assert_int_equal(memoryGetAllocatedCount(), 0);
|
||||
}
|
||||
|
||||
static void test_cache_insert_evictsLeastRecentlyUsed(void **state) {
|
||||
assetlocalefile_t locale;
|
||||
memoryZero(&locale, sizeof(locale));
|
||||
|
||||
// Fill the cache completely, "id0" first (so it's the first to become
|
||||
// least-recently-used) through "id127" last (most-recently-used).
|
||||
char_t key[32], value[32];
|
||||
for(int32_t i = 0; i < ASSET_LOCALE_CACHE_COUNT; i++) {
|
||||
stringFormat(key, sizeof(key), "id%d", i);
|
||||
stringFormat(value, sizeof(value), "value%d", i);
|
||||
assetLocaleCacheInsert(&locale, key, 0, value);
|
||||
}
|
||||
|
||||
// One more insert should evict "id0", the least-recently-used entry.
|
||||
assetLocaleCacheInsert(&locale, "newcomer", 0, "new value");
|
||||
|
||||
char_t result[32];
|
||||
bool_t hit = true;
|
||||
errorret_t ret = assetLocaleCacheFind(&locale, "id0", 0, result, sizeof(result), &hit);
|
||||
assert_true(errorIsOk(ret));
|
||||
assert_false(hit);
|
||||
|
||||
// But "id1" (and everything after it) should have survived.
|
||||
ret = assetLocaleCacheFind(&locale, "id1", 0, result, sizeof(result), &hit);
|
||||
assert_true(errorIsOk(ret));
|
||||
assert_true(hit);
|
||||
assert_string_equal(result, "value1");
|
||||
|
||||
memoryFree(locale.cache);
|
||||
}
|
||||
|
||||
static void test_cache_find_movesHitToFront(void **state) {
|
||||
assetlocalefile_t locale;
|
||||
memoryZero(&locale, sizeof(locale));
|
||||
|
||||
// Fill the cache, then immediately re-touch "id0" so it becomes
|
||||
// most-recently-used instead of least-recently-used.
|
||||
char_t key[32], value[32];
|
||||
for(int32_t i = 0; i < ASSET_LOCALE_CACHE_COUNT; i++) {
|
||||
stringFormat(key, sizeof(key), "id%d", i);
|
||||
stringFormat(value, sizeof(value), "value%d", i);
|
||||
assetLocaleCacheInsert(&locale, key, 0, value);
|
||||
}
|
||||
|
||||
char_t result[32];
|
||||
bool_t hit = false;
|
||||
errorret_t ret = assetLocaleCacheFind(&locale, "id0", 0, result, sizeof(result), &hit);
|
||||
assert_true(errorIsOk(ret));
|
||||
assert_true(hit);
|
||||
|
||||
// Without the touch above, this insert would have evicted "id0" (the
|
||||
// original least-recently-used entry) - it should now evict "id1" instead.
|
||||
assetLocaleCacheInsert(&locale, "newcomer", 0, "new value");
|
||||
|
||||
ret = assetLocaleCacheFind(&locale, "id0", 0, result, sizeof(result), &hit);
|
||||
assert_true(errorIsOk(ret));
|
||||
assert_true(hit);
|
||||
|
||||
ret = assetLocaleCacheFind(&locale, "id1", 0, result, sizeof(result), &hit);
|
||||
assert_true(errorIsOk(ret));
|
||||
assert_false(hit);
|
||||
|
||||
memoryFree(locale.cache);
|
||||
}
|
||||
|
||||
static void test_cache_nullAsserts(void **state) {
|
||||
assetlocalefile_t locale;
|
||||
memoryZero(&locale, sizeof(locale));
|
||||
char_t result[64];
|
||||
bool_t hit;
|
||||
|
||||
expect_assert_failure(
|
||||
assetLocaleCacheFind(NULL, "greeting", 0, result, sizeof(result), &hit)
|
||||
);
|
||||
expect_assert_failure(
|
||||
assetLocaleCacheFind(&locale, NULL, 0, result, sizeof(result), &hit)
|
||||
);
|
||||
expect_assert_failure(
|
||||
assetLocaleCacheFind(&locale, "greeting", 0, result, sizeof(result), NULL)
|
||||
);
|
||||
|
||||
expect_assert_failure(assetLocaleCacheInsert(NULL, "greeting", 0, "value"));
|
||||
expect_assert_failure(assetLocaleCacheInsert(&locale, NULL, 0, "value"));
|
||||
expect_assert_failure(assetLocaleCacheInsert(&locale, "greeting", 0, NULL));
|
||||
}
|
||||
|
||||
static void test_localeDispose_freesCache(void **state) {
|
||||
zip_error_t err;
|
||||
zip_error_init(&err);
|
||||
zip_source_t *write_src = zip_source_buffer_create(NULL, 0, 1, &err);
|
||||
assert_non_null(write_src);
|
||||
zip_t *za = zip_open_from_source(write_src, ZIP_TRUNCATE, &err);
|
||||
assert_non_null(za);
|
||||
size_t flen = strlen(LOCALE_EN);
|
||||
zip_source_t *fs = zip_source_buffer(za, LOCALE_EN, flen, 0);
|
||||
assert_true(zip_file_add(za, "en.locale", fs, ZIP_FL_OVERWRITE) >= 0);
|
||||
zip_source_keep(write_src);
|
||||
assert_int_equal(zip_close(za), 0);
|
||||
|
||||
zip_stat_t zs;
|
||||
memset(&zs, 0, sizeof(zs));
|
||||
assert_int_equal(zip_source_stat(write_src, &zs), 0);
|
||||
void *zipbuf = malloc((size_t)zs.size);
|
||||
assert_non_null(zipbuf);
|
||||
assert_int_equal(zip_source_open(write_src), 0);
|
||||
zip_source_read(write_src, zipbuf, (zip_uint64_t)zs.size);
|
||||
zip_source_close(write_src);
|
||||
zip_source_free(write_src);
|
||||
|
||||
zip_error_init(&err);
|
||||
zip_source_t *read_src = zip_source_buffer_create(zipbuf, (zip_uint64_t)zs.size, 1, &err);
|
||||
assert_non_null(read_src);
|
||||
zip_t *zip = zip_open_from_source(read_src, 0, &err);
|
||||
assert_non_null(zip);
|
||||
ASSET.zip = zip;
|
||||
|
||||
assetentry_t entry;
|
||||
memoryZero(&entry, sizeof(entry));
|
||||
entry.type = ASSET_LOADER_TYPE_LOCALE;
|
||||
|
||||
errorret_t ret = assetFileInit(&entry.data.locale.file, "en.locale", NULL, NULL);
|
||||
assert_true(errorIsOk(ret));
|
||||
ret = assetFileOpen(&entry.data.locale.file);
|
||||
assert_true(errorIsOk(ret));
|
||||
|
||||
char_t header[512];
|
||||
ret = assetLocaleGetString(&entry.data.locale, "", 0, header, sizeof(header));
|
||||
assert_true(errorIsOk(ret));
|
||||
assert_non_null(entry.data.locale.cache);// the header lookup populated it
|
||||
|
||||
ret = assetLocaleDispose(&entry);
|
||||
assert_true(errorIsOk(ret));
|
||||
assert_null(entry.data.locale.cache);
|
||||
assert_int_equal(memoryGetAllocatedCount(), 0);
|
||||
|
||||
zip_close(zip);
|
||||
ASSET.zip = NULL;
|
||||
}
|
||||
|
||||
// ============================================================
|
||||
// assetLocaleGetStringWithArgs - ZIP-based tests
|
||||
// ============================================================
|
||||
@@ -369,7 +638,7 @@ static void test_getStringWithArgs_int(void **state) {
|
||||
|
||||
assert_true(errorIsOk(ret));
|
||||
assert_string_equal(result, "Score: 42");
|
||||
assert_int_equal(memoryGetAllocatedCount(), 0);
|
||||
assert_int_equal(memoryGetAllocatedCount(), 1);// see test_getString_simple
|
||||
}
|
||||
|
||||
static void test_getStringWithArgs_string(void **state) {
|
||||
@@ -383,7 +652,7 @@ static void test_getStringWithArgs_string(void **state) {
|
||||
|
||||
assert_true(errorIsOk(ret));
|
||||
assert_string_equal(result, "Player: Alice");
|
||||
assert_int_equal(memoryGetAllocatedCount(), 0);
|
||||
assert_int_equal(memoryGetAllocatedCount(), 1);// see test_getString_simple
|
||||
}
|
||||
|
||||
// ============================================================
|
||||
@@ -391,6 +660,7 @@ static void test_getStringWithArgs_string(void **state) {
|
||||
// ============================================================
|
||||
|
||||
int main(void) {
|
||||
assertInit();
|
||||
const struct CMUnitTest tests[] = {
|
||||
// parseHeader - pure
|
||||
cmocka_unit_test(test_parseHeader_english),
|
||||
@@ -418,6 +688,21 @@ int main(void) {
|
||||
// getStringWithArgs - in-memory ZIP
|
||||
cmocka_unit_test_setup_teardown(test_getStringWithArgs_int, locale_setup, locale_teardown),
|
||||
cmocka_unit_test_setup_teardown(test_getStringWithArgs_string, locale_setup, locale_teardown),
|
||||
|
||||
// assetLocaleCacheFind / assetLocaleCacheInsert - pure
|
||||
cmocka_unit_test(test_cache_findMiss_onEmptyCache),
|
||||
cmocka_unit_test(test_cache_insertThenFind_hits),
|
||||
cmocka_unit_test(test_cache_distinguishesByPluralCount),
|
||||
cmocka_unit_test(test_cache_find_missOnDifferentMessageId),
|
||||
cmocka_unit_test(test_cache_find_bufferTooSmall_errors),
|
||||
cmocka_unit_test(test_cache_insert_tooLongMessageId_notCached),
|
||||
cmocka_unit_test(test_cache_insert_tooLongValue_notCached),
|
||||
cmocka_unit_test(test_cache_insert_evictsLeastRecentlyUsed),
|
||||
cmocka_unit_test(test_cache_find_movesHitToFront),
|
||||
cmocka_unit_test(test_cache_nullAsserts),
|
||||
|
||||
// assetLocaleDispose - in-memory ZIP
|
||||
cmocka_unit_test(test_localeDispose_freesCache),
|
||||
};
|
||||
return cmocka_run_group_tests(tests, NULL, NULL);
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user