mirror of
https://github.com/libretro/RetroArch.git
synced 2026-11-08 08:10:30 -10:00
menu: pump a pending playlist read every frame, so it finishes on its own
Third report from this path: opening a playlist showed "No Playlist Entries Available", and backing out and opening the same playlist again showed the entries. Sometimes back did not work on the empty screen either. The missing piece was a pump. ENTRIES_NEED_REFRESH is consumed in exactly one place - generic_menu_entry_action() - which runs when the user presses something. There is no per-frame equivalent, so a displaylist that yielded mid-build stayed unfinished until the next keypress: the placeholder was drawn, nothing advanced the read, and the list only filled when an action happened to drive the rebuild. Re-entering the playlist worked because that WAS the input. menu_driver_iterate() now advances a pending read, once per frame, under the same shared I/O window the directory walks and the scanner use, and rebuilds the list itself the moment the read completes - without waiting for input. With nothing outstanding it costs one branch, so the ordinary case is unchanged. This is only reachable at Android/SAF speed. With local stdio a playlist is read inside the first slice and the yield path is never taken, which is why none of these three reports reproduced on a desktop. The oracle gains a pump lane that runs over the short-read VFS standing in for SAF: it asserts the first slice really does yield - so the lane cannot stop covering its own case - and then advances only by repeated frames, no requests and no input, until the read completes and the requested playlist is installed with its own 600 entries. Removing the advance leaves it pending forever, which is the reported symptom. Verified: strict C89 gates on playlist.c and menu_driver.c; full Linux build links with HAVE_QT=1; the playlist oracle passes plain, twice under ASan+UBSan+LSan and twice under TSan; the playlist manager oracle still passes. Still not exercised here: the menu-side rebuild itself. The lane drives the continue/finish pair the pump calls, not menu_driver_iterate(), because nothing links the real menu yet. A harness that links the shipping objects with only main replaced is the missing piece for that, and this path has now earned it.
This commit is contained in:
1 parent
5f586c6396
commit
63f184f526
4 files changed
+156
No files matched your search
@@ -2472,6 +2472,11 @@ static bool menu_driver_displaylist_push_internal(
|
||||
return false;
|
||||
}
|
||||
|
||||
static bool menu_playlist_within_budget(void *ud)
|
||||
{
|
||||
return task_nbio_slice_within_budget(ud, 0, 0);
|
||||
}
|
||||
|
||||
static bool menu_driver_displaylist_push(
|
||||
struct menu_state *menu_st,
|
||||
settings_t *settings,
|
||||
@@ -8202,6 +8207,57 @@ int generic_menu_entry_action(
|
||||
}
|
||||
|
||||
/* Iterate the menu driver for one frame. */
|
||||
/* Advance work a displaylist left unfinished, and rebuild the list
|
||||
* when it completes.
|
||||
*
|
||||
* ENTRIES_NEED_REFRESH is otherwise consumed only in
|
||||
* generic_menu_entry_action(), i.e. when the user presses something.
|
||||
* Anything that yields mid-build therefore sat unfinished until the
|
||||
* next keypress: a playlist read that did not fit its first slice
|
||||
* showed an empty list, and populated only after backing out and
|
||||
* re-entering. This is the pump that was missing - it runs every
|
||||
* frame, with no input required.
|
||||
*
|
||||
* Only a genuinely pending parse does any work here, so a menu with
|
||||
* nothing outstanding costs one predictable branch. */
|
||||
static void menu_driver_pump_pending(struct menu_state *menu_st,
|
||||
settings_t *settings)
|
||||
{
|
||||
nbio_budget_t b;
|
||||
menu_list_t *menu_list;
|
||||
file_list_t *selection_buf;
|
||||
file_list_t *menu_stack;
|
||||
int r;
|
||||
|
||||
if (!playlist_init_cached_pending())
|
||||
return;
|
||||
|
||||
/* One slice of the shared per-frame I/O window, the same budget
|
||||
* the directory walks and the scanner draw from. */
|
||||
task_nbio_slice_open(&b);
|
||||
r = playlist_init_cached_continue(menu_playlist_within_budget, &b);
|
||||
task_nbio_slice_close(&b);
|
||||
|
||||
if (r == 0)
|
||||
return; /* still reading; come back next frame */
|
||||
|
||||
if (r > 0)
|
||||
playlist_init_cached_finish();
|
||||
|
||||
/* Ready (or failed): rebuild the list now rather than waiting for
|
||||
* the user to press something. */
|
||||
if (!(menu_list = menu_st->entries.list))
|
||||
return;
|
||||
selection_buf = MENU_LIST_GET_SELECTION(menu_list, 0);
|
||||
menu_stack = MENU_LIST_GET(menu_list, 0);
|
||||
|
||||
if (selection_buf && menu_stack)
|
||||
menu_driver_displaylist_push(menu_st, settings,
|
||||
selection_buf, menu_stack);
|
||||
|
||||
menu_st->flags &= ~MENU_ST_FLAG_ENTRIES_NEED_REFRESH;
|
||||
}
|
||||
|
||||
bool menu_driver_iterate(
|
||||
struct menu_state *menu_st,
|
||||
gfx_display_t *p_disp,
|
||||
@@ -8210,6 +8266,8 @@ bool menu_driver_iterate(
|
||||
enum menu_action action,
|
||||
retro_time_t current_time)
|
||||
{
|
||||
menu_driver_pump_pending(menu_st, settings);
|
||||
|
||||
return ( menu_st->driver_data
|
||||
&& generic_menu_iterate(
|
||||
menu_st,
|
||||
|
||||
@@ -3996,6 +3996,12 @@ static bool playlist_config_matches(const playlist_config_t *a,
|
||||
&& (a->autofix_paths == b->autofix_paths);
|
||||
}
|
||||
|
||||
/* True while a deferred cached init is part way through a read. */
|
||||
bool playlist_init_cached_pending(void)
|
||||
{
|
||||
return playlist_cached_pending != NULL;
|
||||
}
|
||||
|
||||
void playlist_init_cached_defer_abort(void)
|
||||
{
|
||||
if (playlist_cached_pending)
|
||||
|
||||
@@ -220,6 +220,7 @@ void playlist_parse_abort(playlist_parse_t *p);
|
||||
* playlist_init_cached_defer_abort() abandons any pending parse. */
|
||||
int playlist_init_cached_deferred(const playlist_config_t *config,
|
||||
bool (*budget_cb)(void *), void *budget_ud);
|
||||
bool playlist_init_cached_pending(void);
|
||||
int playlist_init_cached_continue(bool (*budget_cb)(void *), void *budget_ud);
|
||||
int playlist_init_cached_finish(void);
|
||||
void playlist_init_cached_defer_abort(void);
|
||||
|
||||
@@ -1192,6 +1192,96 @@ static void lane_saf_slow_reads(void)
|
||||
yields, saf_read_calls);
|
||||
}
|
||||
|
||||
|
||||
/* The pump: a yielded parse must finish without user input.
|
||||
*
|
||||
* ENTRIES_NEED_REFRESH is consumed only in
|
||||
* generic_menu_entry_action(), so before menu_driver_iterate() got a
|
||||
* pump, a read that yielded stayed unfinished until the next
|
||||
* keypress - the playlist came up empty and populated only after
|
||||
* backing out and re-entering. This drives the same
|
||||
* continue/finish pair the pump calls, over the slow VFS, and
|
||||
* asserts that repeated frames alone complete it. */
|
||||
static void lane_pump_completes_without_input(void)
|
||||
{
|
||||
char path_a[512];
|
||||
char path_b[512];
|
||||
char *doc = NULL;
|
||||
playlist_config_t cfg_a;
|
||||
playlist_config_t cfg_b;
|
||||
playlist_t *cached = NULL;
|
||||
unsigned had = failures;
|
||||
unsigned frames = 0;
|
||||
int r;
|
||||
|
||||
if (!(doc = big_fixture_doc(600, "/games/pump")))
|
||||
{
|
||||
CHECK(false, "pump lane: fixture alloc");
|
||||
return;
|
||||
}
|
||||
snprintf(path_a, sizeof(path_a), "%s/pump_a.lpl", fixture_dir);
|
||||
snprintf(path_b, sizeof(path_b), "%s/pump_b.lpl", fixture_dir);
|
||||
CHECK(write_whole(path_a, doc), "fixture write");
|
||||
CHECK(write_whole(path_b, doc), "fixture write");
|
||||
free(doc);
|
||||
|
||||
config_defaults(&cfg_a, path_a);
|
||||
cfg_a.capacity = 8192;
|
||||
playlist_config_set_base_content_directory(&cfg_a, "/games/pump");
|
||||
config_defaults(&cfg_b, path_b);
|
||||
cfg_b.capacity = 8192;
|
||||
playlist_config_set_base_content_directory(&cfg_b, "/games/pump");
|
||||
|
||||
playlist_free_cached();
|
||||
saf_vfs_install();
|
||||
|
||||
CHECK(playlist_init_cached(&cfg_a), "pump lane: first init");
|
||||
|
||||
/* The first touch, as the displaylist makes it: one budgeted
|
||||
* slice, which over SAF-speed reads does not finish. */
|
||||
{
|
||||
int k = 2;
|
||||
r = playlist_init_cached_deferred(&cfg_b, budget_countdown, &k);
|
||||
}
|
||||
CHECK(r == 0,
|
||||
"pump lane: the first slice finished the read, so this lane "
|
||||
"is not exercising the case it exists for");
|
||||
CHECK(playlist_init_cached_pending(),
|
||||
"pump lane: nothing reported as pending after a yield");
|
||||
|
||||
/* Now only frames - no further requests, no input at all. */
|
||||
while (playlist_init_cached_pending() && frames < 10000)
|
||||
{
|
||||
int k = 2;
|
||||
frames++;
|
||||
r = playlist_init_cached_continue(budget_countdown, &k);
|
||||
if (r > 0)
|
||||
playlist_init_cached_finish();
|
||||
else if (r < 0)
|
||||
break;
|
||||
}
|
||||
|
||||
CHECK(frames > 0, "pump lane: no frames were needed");
|
||||
CHECK(!playlist_init_cached_pending(),
|
||||
"pump lane: still pending after %u frames - a read that "
|
||||
"yields never completes on its own", frames);
|
||||
|
||||
cached = playlist_get_cached();
|
||||
CHECK(cached && streq(playlist_get_conf_path(cached), path_b),
|
||||
"pump lane: the pump did not install the requested "
|
||||
"playlist");
|
||||
CHECK(cached && playlist_size(cached) == 600,
|
||||
"pump lane: %u entries, wanted 600",
|
||||
cached ? (unsigned)playlist_size(cached) : 0);
|
||||
|
||||
saf_vfs_remove();
|
||||
playlist_free_cached();
|
||||
|
||||
if (failures == had)
|
||||
fprintf(stderr, "[pass] pump lane (%u frames, no input)\n",
|
||||
frames);
|
||||
}
|
||||
|
||||
int main(int argc, char *argv[])
|
||||
{
|
||||
char cmd[600];
|
||||
@@ -1224,6 +1314,7 @@ int main(int argc, char *argv[])
|
||||
lane_switch_playlist_never_shows_previous();
|
||||
lane_switch_supersedes_pending();
|
||||
lane_saf_slow_reads();
|
||||
lane_pump_completes_without_input();
|
||||
|
||||
snprintf(cmd, sizeof(cmd), "rm -rf %s", fixture_dir);
|
||||
if (system(cmd) != 0) { }
|
||||
|
||||
Reference in new issue
Block a user