From d6f7b3d4013a6f2bcd79f3a6df4812182f36e5bf Mon Sep 17 00:00:00 2001 From: "Alex M. Wells" Date: Mon, 3 Mar 2025 13:48:12 -0800 Subject: [PATCH 1/4] Share Shading Context when optimizing a shader vs. grabbing an additional context. No need for a 2nd set of heaps/buffers when the existing context can be reused. Reduces memory footprint which increases cache efficiency, especially for threaded usage. Signed-off-by: Alex M. Wells --- src/liboslexec/context.cpp | 33 ++++++++++++++++++++++----------- 1 file changed, 22 insertions(+), 11 deletions(-) diff --git a/src/liboslexec/context.cpp b/src/liboslexec/context.cpp index 8331924a7..2b3b4dbe4 100644 --- a/src/liboslexec/context.cpp +++ b/src/liboslexec/context.cpp @@ -86,14 +86,21 @@ ShadingContext::execute_init(ShaderGroup& sgroup, int threadindex, if (sgroup.nlayers()) { sgroup.start_running(); if (!sgroup.jitted()) { - auto ctx = shadingsys().get_context(thread_info()); - shadingsys().optimize_group(sgroup, ctx, true /*do_jit*/); + // Reuse "this" ShadingContext vs creating a new one, + // but match previous behavior of default null texture thread info + // as if we created a new ShadingContext + process_errors(); // process any buffered errors before reusing "this" context + auto existing_tti = texture_thread_info(); + texture_thread_info(nullptr); + shadingsys().optimize_group(sgroup, this, true /*do_jit*/); if (shadingsys().m_greedyjit && shadingsys().m_groups_to_compile_count) { // If we are greedily JITing, optimize/JIT everything now shadingsys().optimize_all_groups(); } - shadingsys().release_context(ctx); + // Restore "this" ShadingContext + process_errors(); // process any buffered errors before restoring "this" context + texture_thread_info(existing_tti); } if (sgroup.does_nothing()) return false; @@ -260,19 +267,23 @@ ShadingContext::Batched::execute_init( if (sgroup.nlayers()) { sgroup.start_running(); if (!sgroup.batch_jitted()) { - // Matching ShadingContext::execute_init behavior - // of grabbing another context. - // TODO: Is this necessary, why can't we just use the - // the existing context()? - //shadingsys().template batched().jit_group(sgroup, &context()); - auto ctx = shadingsys().get_context(context().thread_info()); - shadingsys().template batched().jit_group(sgroup, ctx); + // Reuse "this" ShadingContext vs creating a new one, + // but match previous behavior of default null texture thread info + // as if we created a new ShadingContext + context().process_errors(); // process any buffered errors before reusing "this" context + auto existing_tti = context().texture_thread_info(); + context().texture_thread_info(nullptr); + shadingsys().template batched().jit_group(sgroup, &context()); + if (shadingsys().m_greedyjit && shadingsys().m_groups_to_compile_count) { // If we are greedily JITing, optimize/JIT everything now shadingsys().template batched().jit_all_groups(); } - shadingsys().release_context(ctx); + + // Restore "this" ShadingContext + context().process_errors(); // process any buffered errors before restoring "this" context + context().texture_thread_info(existing_tti); } // To handle layers that were not used but still possibly had // render outputs, we always generate a run function even for From f58c4c987a5199489a40869e567d8dbbf6f06310 Mon Sep 17 00:00:00 2001 From: "Alex M. Wells" Date: Mon, 3 Mar 2025 14:00:24 -0800 Subject: [PATCH 2/4] clang format changes Signed-off-by: Alex M. Wells --- src/liboslexec/context.cpp | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/src/liboslexec/context.cpp b/src/liboslexec/context.cpp index 2b3b4dbe4..bda35f0b3 100644 --- a/src/liboslexec/context.cpp +++ b/src/liboslexec/context.cpp @@ -89,7 +89,7 @@ ShadingContext::execute_init(ShaderGroup& sgroup, int threadindex, // Reuse "this" ShadingContext vs creating a new one, // but match previous behavior of default null texture thread info // as if we created a new ShadingContext - process_errors(); // process any buffered errors before reusing "this" context + process_errors(); // process any buffered errors before reusing "this" context auto existing_tti = texture_thread_info(); texture_thread_info(nullptr); shadingsys().optimize_group(sgroup, this, true /*do_jit*/); @@ -99,7 +99,7 @@ ShadingContext::execute_init(ShaderGroup& sgroup, int threadindex, shadingsys().optimize_all_groups(); } // Restore "this" ShadingContext - process_errors(); // process any buffered errors before restoring "this" context + process_errors(); // process any buffered errors before restoring "this" context texture_thread_info(existing_tti); } if (sgroup.does_nothing()) @@ -270,19 +270,22 @@ ShadingContext::Batched::execute_init( // Reuse "this" ShadingContext vs creating a new one, // but match previous behavior of default null texture thread info // as if we created a new ShadingContext - context().process_errors(); // process any buffered errors before reusing "this" context + context() + .process_errors(); // process any buffered errors before reusing "this" context auto existing_tti = context().texture_thread_info(); context().texture_thread_info(nullptr); - shadingsys().template batched().jit_group(sgroup, &context()); + shadingsys().template batched().jit_group(sgroup, + &context()); if (shadingsys().m_greedyjit && shadingsys().m_groups_to_compile_count) { // If we are greedily JITing, optimize/JIT everything now shadingsys().template batched().jit_all_groups(); } - + // Restore "this" ShadingContext - context().process_errors(); // process any buffered errors before restoring "this" context + context() + .process_errors(); // process any buffered errors before restoring "this" context context().texture_thread_info(existing_tti); } // To handle layers that were not used but still possibly had From 0d4a79800552ef680fa8f46cb26b406e9349e9a7 Mon Sep 17 00:00:00 2001 From: "Alex M. Wells" Date: Tue, 4 Mar 2025 13:15:31 -0800 Subject: [PATCH 3/4] Added "RestoreState ShadingContext::repurposeForJit()" and "void ShadingContext::restoreFromJit(const RestoreState &)" to reduce repeated code and make usage/intent more self documenting Signed-off-by: Alex M. Wells --- src/liboslexec/context.cpp | 44 +++++++++++++++++++----------------- src/liboslexec/oslexec_pvt.h | 12 ++++++++++ 2 files changed, 35 insertions(+), 21 deletions(-) diff --git a/src/liboslexec/context.cpp b/src/liboslexec/context.cpp index bda35f0b3..9389bc623 100644 --- a/src/liboslexec/context.cpp +++ b/src/liboslexec/context.cpp @@ -68,6 +68,25 @@ ShadingContext::~ShadingContext() free_dict_resources(); } +ShadingContext::RestoreState +ShadingContext::repurposeForJit() +{ + process_errors(); + // Match previous behavior of nullptr value for texture thread info + // as if we created a new ShadingContext + RestoreState restore_state{texture_thread_info()}; + texture_thread_info(nullptr); + return restore_state; +} + + // Process any errors from JIT and restore the texture_thread_info; +void +ShadingContext::restoreFromJit(const RestoreState &restore_state) +{ + // Restore "this" ShadingContext for execution + process_errors(); + texture_thread_info(restore_state.m_pre_jit_texture_thread_info); +} bool @@ -86,21 +105,14 @@ ShadingContext::execute_init(ShaderGroup& sgroup, int threadindex, if (sgroup.nlayers()) { sgroup.start_running(); if (!sgroup.jitted()) { - // Reuse "this" ShadingContext vs creating a new one, - // but match previous behavior of default null texture thread info - // as if we created a new ShadingContext - process_errors(); // process any buffered errors before reusing "this" context - auto existing_tti = texture_thread_info(); - texture_thread_info(nullptr); + auto restore_state = repurposeForJit(); shadingsys().optimize_group(sgroup, this, true /*do_jit*/); if (shadingsys().m_greedyjit && shadingsys().m_groups_to_compile_count) { // If we are greedily JITing, optimize/JIT everything now shadingsys().optimize_all_groups(); } - // Restore "this" ShadingContext - process_errors(); // process any buffered errors before restoring "this" context - texture_thread_info(existing_tti); + restoreFromJit(restore_state); } if (sgroup.does_nothing()) return false; @@ -267,13 +279,7 @@ ShadingContext::Batched::execute_init( if (sgroup.nlayers()) { sgroup.start_running(); if (!sgroup.batch_jitted()) { - // Reuse "this" ShadingContext vs creating a new one, - // but match previous behavior of default null texture thread info - // as if we created a new ShadingContext - context() - .process_errors(); // process any buffered errors before reusing "this" context - auto existing_tti = context().texture_thread_info(); - context().texture_thread_info(nullptr); + auto restore_state = context().repurposeForJit(); shadingsys().template batched().jit_group(sgroup, &context()); @@ -282,11 +288,7 @@ ShadingContext::Batched::execute_init( // If we are greedily JITing, optimize/JIT everything now shadingsys().template batched().jit_all_groups(); } - - // Restore "this" ShadingContext - context() - .process_errors(); // process any buffered errors before restoring "this" context - context().texture_thread_info(existing_tti); + context().restoreFromJit(restore_state); } // To handle layers that were not used but still possibly had // render outputs, we always generate a run function even for diff --git a/src/liboslexec/oslexec_pvt.h b/src/liboslexec/oslexec_pvt.h index fa2a95628..e0ad2a1c1 100644 --- a/src/liboslexec/oslexec_pvt.h +++ b/src/liboslexec/oslexec_pvt.h @@ -2518,6 +2518,18 @@ class OSLEXECPUBLIC ShadingContext { // wide data offsets should be used int batch_size_executed; bool execution_is_batched() const { return batch_size_executed != 0; } + + + struct RestoreState { + TextureSystem::Perthread* m_pre_jit_texture_thread_info; + }; + + // Rather than allocate an addtional ShadingContext for JIT + // reuse this one by processing any existing errors + // and saving off any necessary state to be restored afterwards + RestoreState repurposeForJit(); + // Process any errors from JIT and restore the state + void restoreFromJit(const RestoreState &); }; From 484f5e143dee48081246840fd8b7619b75ae4dac Mon Sep 17 00:00:00 2001 From: "Alex M. Wells" Date: Tue, 4 Mar 2025 13:25:26 -0800 Subject: [PATCH 4/4] clang format changes Signed-off-by: Alex M. Wells --- src/liboslexec/context.cpp | 10 +++++----- src/liboslexec/oslexec_pvt.h | 6 +++--- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/liboslexec/context.cpp b/src/liboslexec/context.cpp index 9389bc623..2939ad09a 100644 --- a/src/liboslexec/context.cpp +++ b/src/liboslexec/context.cpp @@ -71,20 +71,20 @@ ShadingContext::~ShadingContext() ShadingContext::RestoreState ShadingContext::repurposeForJit() { - process_errors(); + process_errors(); // Match previous behavior of nullptr value for texture thread info // as if we created a new ShadingContext - RestoreState restore_state{texture_thread_info()}; + RestoreState restore_state { texture_thread_info() }; texture_thread_info(nullptr); return restore_state; } - // Process any errors from JIT and restore the texture_thread_info; +// Process any errors from JIT and restore the texture_thread_info void -ShadingContext::restoreFromJit(const RestoreState &restore_state) +ShadingContext::restoreFromJit(const RestoreState& restore_state) { // Restore "this" ShadingContext for execution - process_errors(); + process_errors(); texture_thread_info(restore_state.m_pre_jit_texture_thread_info); } diff --git a/src/liboslexec/oslexec_pvt.h b/src/liboslexec/oslexec_pvt.h index e0ad2a1c1..145f5b275 100644 --- a/src/liboslexec/oslexec_pvt.h +++ b/src/liboslexec/oslexec_pvt.h @@ -2521,15 +2521,15 @@ class OSLEXECPUBLIC ShadingContext { struct RestoreState { - TextureSystem::Perthread* m_pre_jit_texture_thread_info; + TextureSystem::Perthread* m_pre_jit_texture_thread_info; }; - // Rather than allocate an addtional ShadingContext for JIT + // Rather than allocate an additional ShadingContext for JIT // reuse this one by processing any existing errors // and saving off any necessary state to be restored afterwards RestoreState repurposeForJit(); // Process any errors from JIT and restore the state - void restoreFromJit(const RestoreState &); + void restoreFromJit(const RestoreState&); };