PERF: Minimal, correct Python wrapping DEPENDS under lazy VTK loading - #71
Merged
Merged
Conversation
…ed methods The Python module initialization generated by vtkMacroKitPythonWrap listed a library's Slicer dependencies in the init's vtkPythonUtil::ImportModule list but deliberately excluded its VTK dependencies. That worked only because importing "vtk" eagerly loaded every VTK module, so a wrapped subclass's VTK base class was always registered by the time its Python type was built. With lazy VTK loading (a "vtk" shim that imports vtkmodules submodules on demand), the VTK base module is no longer guaranteed to be loaded first. A wrapped subclass such as vtkOrientedGridTransform (whose base vtkGridTransform lives in vtkmodules.vtkFiltersHybrid) is then built with base "object" and silently loses every inherited method -- for example SetDisplacementGridData -- until something else happens to import the base module. Emit the VTK dependencies in the module init's import list too, mirroring VTK's own module wrapper (vtkModuleWrapPython): skip modules marked exclude_wrap and name the rest "<python_package>.<library_name>" (e.g. vtkmodules.vtkFiltersHybrid). Each library wrapped with this macro -- vtkAddon, and every Slicer or extension library using it -- then imports its VTK base modules when it loads, so lazy VTK loading no longer breaks cross-module inheritance. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
vtk_wrap_python seeds each wrapped module's Python dependencies from the whole
VTK link line (${VTK_LIBRARIES}), so every "vtk*Init.data" gains a DEPENDS on
all ~125 VTK Python modules. Importing a base class before its wrapped subclass
is built only requires the module that *provides* that base (each VTK module
imports its own bases transitively), so listing the full link line is both
unnecessary and, with lazy VTK loading, harmful: importing any wrapped module
would pull in all of VTK at startup.
Add SlicerPrunePythonModuleDepends.py and run it as a build step: it reads the
merged wrapping hierarchy (Class : Super ; header ; owning_module) and keeps
only the vtkmodules.* DEPENDS whose module owns a direct base class of one of
the module's wrapped classes, passing cross-module Slicer dependencies through
unchanged. The pruning is conservative -- only spurious VTK modules are removed
-- and falls back to the unpruned data file when the hierarchy file or a Python
interpreter is unavailable.
Applied across the tree the DEPENDS drop from 7223 to 520 entries over 54
modules with no module left empty. vtkAddonPython goes from 125 to 7 direct
DEPENDS (16 vs 125 modules loaded transitively), while the inheritance fix is
preserved: vtkOrientedGridTransform keeps its full method resolution order and
SetDisplacementGridData. This makes the restored base-class DEPENDS coexist
with lazy VTK loading instead of defeating it.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
A module's Python wrapping hierarchy must be generated after the hierarchies of the modules it wraps against. VTK_WRAP_HIERARCHY collected those as target-level dependencies (OTHER_HIERARCHY_TARGETS) only for hierarchy targets that already existed when the module was configured. A dependency configured *later* in the subdirectory order -- a forward reference, e.g. Transforms and Segmentations wrapping against Markups MRML -- had no "<dep>Hierarchy" target yet, so on the Makefiles generator that build-order edge was silently dropped and parallel builds could race with: vtkWrapHierarchy: couldn't open file .../vtkSlicerMarkupsModuleMRMLHierarchy.txt Incremental build trees masked it (the file was left over from a prior build); fresh parallel trees hit it depending on scheduling. Defer the dependency wiring with cmake_language(DEFER) to the end of the top-level directory, where every module has been configured and all hierarchy targets exist, and add each edge there (guarded on both targets existing). This handles forward and backward references uniformly and removes the need for projects to re-add known forward dependencies by hand after configuring all their modules. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
pieper
force-pushed
the
fix-lazy-vtk-wrapped-inheritance
branch
from
July 31, 2026 15:55
6a34fe7 to
98958af
Compare
pieper
changed the title
pieper
marked this pull request as ready for review
August 5, 2026 19:40
Member
Author
|
If you can merge this and some others I'll take out of draft then I'll check the builds tomorrow and chase down any issues. |
This was referenced Aug 5, 2026
lassoan
approved these changes
Aug 7, 2026
lassoan
left a comment
lassoan
left a comment
Contributor
There was a problem hiding this comment.
While I haven't fully reviewed all the details, the changes overall make sense, and if issues come up then we can tweak these further.
Contributor
|
This may have broken then Linux preview build: https://slicer.cdash.org/builds/4351371/build
UPDATE: I am wrong, #67 is responsible! |
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
3 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.
Three related fixes to
vtkMacroKitPythonWrapso Slicer's wrapped C++ classeswork correctly and load minimally under the lazy
vtkshim(Slicer/Slicer#9306).
1. Restore base-class module imports (BUG)
The eager
import vtkused to load every VTK module up front. With lazy VTKloading it does not, so a wrapped subclass whose VTK base class had not been
imported was built without that base and silently lost the inherited methods
(e.g.
vtkOrientedGridTransform.SetDisplacementGridData). Emit, for each VTKdependency, the
<python_package>.<library_name>module in the generatedDEPENDS, following VTK's ownvtkModuleWrapPython.2. Prune DEPENDS to base-class providers (PERF)
Those DEPENDS are seeded from the whole
${VTK_LIBRARIES}link line, so everyvtk*Init.datagained a DEPENDS on all ~125 VTK Python modules — importing anywrapped module would pull in all of VTK at startup, defeating lazy loading. A
wrapped subclass only needs its direct base class registered first (each VTK
module imports its own bases transitively), so
SlicerPrunePythonModuleDepends.pyreads the merged wrapping hierarchy and keeps only the
vtkmodules.*DEPENDSwhose module owns a direct base class of a wrapped class. Cross-module Slicer
dependencies are preserved unchanged; conservative (only spurious VTK modules
are removed) with a fallback to the unpruned file when the hierarchy or a Python
interpreter is unavailable.
Applied across Slicer the DEPENDS drop from 7223 to 520 entries over 54 modules,
none left empty;
vtkAddonPythongoes from 125 to 7 direct DEPENDS (16 vs 125modules loaded transitively).
3. Wire hierarchy build-order dependencies generically (BUG)
VTK_WRAP_HIERARCHYonly added a build-order dependency on the hierarchytargets it wraps against when those targets already existed at configure time.
A dependency configured later in the subdirectory order (a forward reference)
had no target yet, so on the Makefiles generator the edge was silently dropped
and parallel builds could race with
vtkWrapHierarchy: couldn't open file ...Hierarchy.txt. Defer the wiring withcmake_language(DEFER)to the end ofthe top-level directory, where all hierarchy targets exist. This makes the
per-project workaround in Slicer/Slicer#9305 unnecessary.
Verification
isinstance/mro restored across all wrapped subclasses; a full from-scratchRelease build loads 29 of 125 VTK Python modules at GUI startup (none of the
rendering/IO/Charts tail); warm startup ~5.9s → 4.9s on an M-series Mac.
Pairs with the Slicer-side change that removes the now-obsolete hardcoded
base-class import list.