Skip to content

Conversation

@AlexMWells
Copy link
Contributor

… vs. grabbing an additi onal context.

Description

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.

We can just swap the TexturePerThread info out for a null pointer and restore the previous one afterwards
For completeness we also process the errors in the context.

Tests

Existing tests exercise the changes.

Checklist:

  • I have read the contribution guidelines.
  • I have updated the documentation, if applicable.
  • I have ensured that the change is tested somewhere in the testsuite (adding new test cases if necessary).
  • My code follows the prevailing code style of this project. If I haven't
    already run clang-format v17 before submitting, I definitely will look at
    the CI test that runs clang-format and fix anything that it highlights as
    being nonconforming.

…onal 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 <[email protected]>
@AlexMWells AlexMWells changed the title Share Shading Context when optimizing a shader… Share Shading Context when optimizing/jitting a shader… Mar 3, 2025
Signed-off-by: Alex M. Wells <[email protected]>
@AlexMWells
Copy link
Contributor Author

Hold up on this, going to update so that such that the work to "borrow" a shading context for opt/jit is internal to ShadingContext and not burden on its user to understand the implementation details.

@lgritz
Copy link
Collaborator

lgritz commented Mar 4, 2025

#1953 fixes the broken CI, which is not the fault of this PR. I merged it, so if you rebase on the current main, you should get a passing CI.

…ingContext::restoreFromJit(const RestoreState &)" to reduce repeated code and make usage/intent more self documenting

Signed-off-by: Alex M. Wells <[email protected]>
Signed-off-by: Alex M. Wells <[email protected]>
@AlexMWells
Copy link
Contributor Author

Ok Larry, this is ready for review

Copy link
Collaborator

@lgritz lgritz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for the delay, this slipped through the cracks. LGTM.

@lgritz lgritz merged commit 532053e into AcademySoftwareFoundation:main Jul 8, 2025
26 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants