Cleanup after instance cache rework.#6209
Merged
mzient merged 5 commits intoNVIDIA:mainfrom Feb 23, 2026
Merged
Conversation
Signed-off-by: Michal Zientkiewicz <michalz@nvidia.com>
Signed-off-by: Michal Zientkiewicz <michalz@nvidia.com>
Collaborator
|
CI MESSAGE: [43973595]: BUILD STARTED |
Contributor
Greptile OverviewGreptile SummaryThis PR addresses code cleanup issues identified in the review of PR #6206, focusing on proper resource management and documentation improvements. Key changes:
Confidence Score: 5/5
Important Files Changed
Last reviewed commit: da3b5bb |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| def __del__(self): | ||
| self._return_op_to_cache() | ||
| if hasattr(self._operator, "_cache"): | ||
| self._return_op_to_cache = weakref.finalize(self, self._return_op_to_cache_impl) |
Contributor
There was a problem hiding this comment.
passing self._return_op_to_cache_impl creates a circular reference - the bound method holds a reference to self, preventing the finalizer from running. need to extract the cache, keyname, and operator instance before creating the finalizer, then pass them as separate arguments to a lambda or standalone function (see how pipeline.py:992 uses lambda with separate args)
Signed-off-by: Michal Zientkiewicz <michalz@nvidia.com>
Collaborator
|
CI MESSAGE: [43975029]: BUILD STARTED |
| @classmethod | ||
| def _process_params(cls, backend, op_device, batch_size, *raw_args, **raw_kwargs): | ||
| """ | ||
| Processes run-time parameters passed to the operator to ones that can be consumed DALI |
Contributor
There was a problem hiding this comment.
typo: 'parmaeters' should be 'parameters'
Suggested change
| Processes run-time parameters passed to the operator to ones that can be consumed DALI | |
| Processes run-time parameters passed to the operator to ones that can be consumed DALI |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Collaborator
|
CI MESSAGE: [43976464]: BUILD STARTED |
Collaborator
|
CI MESSAGE: [43973595]: BUILD PASSED |
Collaborator
|
CI MESSAGE: [43976464]: BUILD FAILED |
Collaborator
|
CI MESSAGE: [43976464]: BUILD PASSED |
banasraf
approved these changes
Feb 16, 2026
mdabek-nvidia
approved these changes
Feb 16, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Category:
Refactoring (Redesign of existing code that doesn't affect functionality)
Description:
This PR addresses low priority issues found in #6206 review.
Additional information:
Affected modules and functionalities:
Invocation
Key points relevant for the review:
weakref.finalizeusageTests:
Checklist
Documentation
DALI team only
Requirements
REQ IDs: N/A
JIRA TASK: N/A