From 4bad0667dd6ddfa1a43339ae37b3535550e8e264 Mon Sep 17 00:00:00 2001 From: Vito Caputo Date: Sat, 8 Jul 2023 20:15:06 -0700 Subject: modules/checkers: stop setting til_frame_plan_t.cpu_affinity In the early days of checkers when I introduced fill_module= with the per-cpu contexts to still allow threaded rendering, the whole seed passing to contexts thing wasn't as well sorted out. This meant the contexts often produced vastly different outputs despite being the same module, same seed, and same settings. The consequence of that was that w/fill_module checkers would produce crazy randomized output when you expected the same output in the filled cells. But by using .cpu_affinity (which I had to implement just for this use case actually) at least the different outputs would become stable. It was a band-aid over a different problem that still needed sorting out. Nowadays, it seems like this is improved enough at least for alphazed to look correct without the affinity hack, so I'm removing it because it really kills checkers threaded performance. Whatever modules remain uncooperative WRT seed reproducibility, they'll just need to be fixed up. --- src/modules/checkers/checkers.c | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) (limited to 'src/modules/checkers') diff --git a/src/modules/checkers/checkers.c b/src/modules/checkers/checkers.c index 5d76a88..f04ebf0 100644 --- a/src/modules/checkers/checkers.c +++ b/src/modules/checkers/checkers.c @@ -224,17 +224,19 @@ static int checkers_fragmenter(til_module_context_t *context, const til_fb_fragm static void checkers_prepare_frame(til_module_context_t *context, til_stream_t *stream, unsigned ticks, til_fb_fragment_t **fragment_ptr, til_frame_plan_t *res_frame_plan) { - checkers_context_t *ctxt = (checkers_context_t *)context; - - /* XXX: note cpu_affinity is required when fill_module is used, to ensure module_contexts - * have a stable relationship to fragnum. Otherwise the output would be unstable because the - * module contexts would be randomly distributed across the filled checkers frame-to-frame. - * This is unfortunate since cpu_affinity is likely to be slower than just letting the render - * threads render fragments in whatever order (the preferred default). fill_module here - * is actually *the* reason til_frame_plan_t.cpu_affinity got implemented, before this there - * wasn't even a til_frame_plan_t container; a bare til_fragmenter_t was returned. + /* XXX: once upon a time this used .cpu_affinity = 1, to + * get a stable mapping of per-cpu-context to filled cell + * via/fill_module. But that slows things down + * substantially as-implemented in til_threads since it + * makes threads wait for their next fragnum to come up. + * Since seed-ifying everything, and the seed is the same + * across the contexts, they _should_ have identical + * outputs. So I got rid of it for the perf gain, and + * modules which aren't behaving well WRT + * same-seeds-same-settings-but-different-output, fix + * them. */ - *res_frame_plan = (til_frame_plan_t){ .fragmenter = checkers_fragmenter, .cpu_affinity = !!ctxt->setup->fill_module }; + *res_frame_plan = (til_frame_plan_t){ .fragmenter = checkers_fragmenter }; } -- cgit v1.2.3