Skip to content

Conversation

@cpcloud
Copy link
Contributor

@cpcloud cpcloud commented Dec 10, 2025

Remove the ctypes linker and inline _LinkerBase methods since there is only a single linker class now.

Also removes _Linker.new since it is now redundant with the class's __init__ method.

@cpcloud
Copy link
Contributor Author

cpcloud commented Dec 10, 2025

/ok to test

@copy-pr-bot
Copy link

copy-pr-bot bot commented Dec 10, 2025

Auto-sync is disabled for ready for review pull requests in this repository. Workflows must be run manually.

Contributors can view more details about this message here.

@cpcloud cpcloud changed the title cull dead objects refactor: cull dead linker objects Dec 10, 2025
@greptile-apps
Copy link
Contributor

greptile-apps bot commented Dec 10, 2025

Greptile Overview

Greptile Summary

This PR successfully removes dead linker code by eliminating the unused CtypesLinker class and the _LinkerBase abstract base class. The refactoring consolidates all linker functionality into a single _Linker class, removing the now-unnecessary .new() factory method in favor of direct constructor calls.

Key changes:

  • Removed ~150 lines of dead code (CtypesLinker class and _LinkerBase metaclass)
  • Inlined abstract methods from _LinkerBase directly into _Linker with proper docstrings
  • Replaced all _Linker.new() calls with direct _Linker() constructor calls
  • Renamed Function class to CudaPythonFunction for better inspectability (with backward compatibility alias)
  • All call sites properly updated with correct parameters

The refactoring is safe and maintains backward compatibility. All existing functionality is preserved.

Confidence Score: 5/5

  • This PR is safe to merge with minimal risk - it's a clean refactoring that removes dead code without changing functionality
  • The changes are straightforward: removing unused classes and inlining methods. All call sites have been properly updated to use the direct constructor instead of the factory method. The _Linker.__init__ signature remains unchanged with appropriate defaults (max_registers=None), ensuring backward compatibility. No logic changes were made, only code organization improvements.
  • No files require special attention

Important Files Changed

File Analysis

Filename Score Overview
numba_cuda/numba/cuda/cudadrv/driver.py 5/5 Removed CtypesLinker class and _LinkerBase abstract base class, inlined methods into _Linker, and renamed Function to CudaPythonFunction for better inspectability
numba_cuda/numba/cuda/codegen.py 5/5 Updated two _Linker.new() calls to use direct _Linker() constructor instead
numba_cuda/numba/cuda/memory_management/nrt.py 5/5 Updated _Linker.new() call to direct _Linker() constructor with explicit max_registers=0 parameter
numba_cuda/numba/cuda/tests/cudadrv/test_linker.py 5/5 Updated test calls from _Linker.new() to direct _Linker() constructor with explicit max_registers=0 parameter

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

Additional Comments (1)

  1. numba_cuda/numba/cuda/cudadrv/driver.py, line 2338-2347 (link)

    logic: The new() method contains dead code. Line 2338 hardcodes linker = _Linker, so the condition if linker is _Linker on line 2341 is always true, making the else block (lines 2343-2345) unreachable.

1 file reviewed, 1 comment

Edit Code Review Agent Settings | Greptile

@cpcloud
Copy link
Contributor Author

cpcloud commented Dec 10, 2025

/ok to test

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

4 files reviewed, no comments

Edit Code Review Agent Settings | Greptile

@cpcloud cpcloud merged commit 206058f into NVIDIA:main Dec 11, 2025
141 of 142 checks passed
@cpcloud cpcloud deleted the cull-dead-objects branch December 11, 2025 00:04
gmarkall added a commit to gmarkall/numba-cuda that referenced this pull request Dec 17, 2025
- Fix NVIDIA#624: Accept Numba IR nodes in all places Numba-CUDA IR nodes are expected (NVIDIA#643)
- Fix Issue NVIDIA#588: separate compilation of NVVM IR modules when generating debuginfo (NVIDIA#591)
- feat: allow printing nested tuples (NVIDIA#667)
- build(deps): bump actions/setup-python from 5.6.0 to 6.1.0 (NVIDIA#655)
- build(deps): bump actions/upload-artifact from 4 to 5 (NVIDIA#652)
- Test RAPIDS 25.12 (NVIDIA#661)
- Do not manually set DUMP_ASSEMBLY in `nvjitlink` tests (NVIDIA#662)
- feat: add print support for int64 tuples (NVIDIA#663)
- Only run dependabot monthly and open fewer PRs (NVIDIA#658)
- test: fix bogus `self` argument to `Context` (NVIDIA#656)
- Fix false negative NRT link decision when NRT was previously toggled on (NVIDIA#650)
- Add support for dependabot (NVIDIA#647)
- refactor: cull dead linker objects (NVIDIA#649)
- Migrate numba-cuda driver to use cuda.core.launch API (NVIDIA#609)
- feat: add set_shared_memory_carveout (NVIDIA#629)
- chore: bump version in pixi.toml (NVIDIA#641)
- refactor: remove devicearray code to reduce complexity (NVIDIA#600)
@gmarkall gmarkall mentioned this pull request Dec 17, 2025
gmarkall added a commit that referenced this pull request Dec 17, 2025
- Capture global device arrays in kernels and device functions (#666)
- Fix #624: Accept Numba IR
nodes in all places Numba-CUDA IR nodes are expected
(#643)
- Fix Issue #588: separate
compilation of NVVM IR modules when generating debuginfo
(#591)
- feat: allow printing nested tuples
(#667)
- build(deps): bump actions/setup-python from 5.6.0 to 6.1.0
(#655)
- build(deps): bump actions/upload-artifact from 4 to 5
(#652)
- Test RAPIDS 25.12 (#661)
- Do not manually set DUMP_ASSEMBLY in `nvjitlink` tests
(#662)
- feat: add print support for int64 tuples
(#663)
- Only run dependabot monthly and open fewer PRs
(#658)
- test: fix bogus `self` argument to `Context`
(#656)
- Fix false negative NRT link decision when NRT was previously toggled
on (#650)
- Add support for dependabot
(#647)
- refactor: cull dead linker objects
(#649)
- Migrate numba-cuda driver to use cuda.core.launch API
(#609)
- feat: add set_shared_memory_carveout
(#629)
- chore: bump version in pixi.toml
(#641)
- refactor: remove devicearray code to reduce complexity
(#600)
ZzEeKkAa added a commit to ZzEeKkAa/numba-cuda that referenced this pull request Jan 8, 2026
v0.23.0

- Capture global device arrays in kernels and device functions (NVIDIA#666)
- Fix NVIDIA#624: Accept Numba IR nodes in all places Numba-CUDA IR nodes are expected (NVIDIA#643)
- Fix Issue NVIDIA#588: separate compilation of NVVM IR modules when generating debuginfo (NVIDIA#591)
- feat: allow printing nested tuples (NVIDIA#667)
- build(deps): bump actions/setup-python from 5.6.0 to 6.1.0 (NVIDIA#655)
- build(deps): bump actions/upload-artifact from 4 to 5 (NVIDIA#652)
- Test RAPIDS 25.12 (NVIDIA#661)
- Do not manually set DUMP_ASSEMBLY in `nvjitlink` tests (NVIDIA#662)
- feat: add print support for int64 tuples (NVIDIA#663)
- Only run dependabot monthly and open fewer PRs (NVIDIA#658)
- test: fix bogus `self` argument to `Context` (NVIDIA#656)
- Fix false negative NRT link decision when NRT was previously toggled on (NVIDIA#650)
- Add support for dependabot (NVIDIA#647)
- refactor: cull dead linker objects (NVIDIA#649)
- Migrate numba-cuda driver to use cuda.core.launch API (NVIDIA#609)
- feat: add set_shared_memory_carveout (NVIDIA#629)
- chore: bump version in pixi.toml (NVIDIA#641)
- refactor: remove devicearray code to reduce complexity (NVIDIA#600)
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