[ITK-dev] ITK+static+wrapping [ref: 21857 "Enable singleton program global timestamp"]

[Isaiah Norton]

Hi Matt,

There is one potential technical issue with the scheme added by 21857. I
think that users who embed Python in a native application, with the app
statically linked to ITK, will need to manually ensure that the
application-side timestamp is initialized to point to the variable from
_ITKCommonPython. Otherwise, any objects initialized from application-side
ITK code would face the same desynchronization issue. Also, as mentioned in
the comments on the merge request, fully supporting the
(ITK_WRAP_PYTHON=ON, BUILD_SHARED_LIBS=OFF) multi-object configuration will
require moving other globals to the new Python side-loaded singleton system.

Given the above, perhaps that build configuration should be an error by
default, for now (with override available), so people don't use it
inadvertently? The problems caused can be difficult to debug.

Best,
Isaiah


On Sat, Dec 17, 2016 at 10:41 AM, Matt McCormick <matt.mccormick at kitware.com
> wrote:

> Hi Isaiah,
>
> Here is some more information and background.
>
>
> > What is the rationale behind creating so many separate, statically-linked
> > Python modules? (78 `_ITK*Python.so` files on my machine). Creating a
> single
> > bundle instead should resolve the original issue, and might also reduce
> the
> > size of the final library [3].
>
> For background, the GlobalTimeStamp, is a very important variable in
> ITK (and VTK). This is an atomic integer that gets incremented when
> any itk::Object (most classes in ITK) is "Modified", i.e. changed to
> note that it should be updated in a processing pipeline. So, almost
> all ITK-based code needs it, and it needs to be global across the
> entire program.
>
> The GlobalTimeStamp lives in and is used my itk::TimeStamp.cxx, and it
> is incremented on every call to ::Modified(). The itk::TimeStamp class
> is linked into libITKCommon.
>
> When ITK is built with shared libraries, libITKCommon is shared, and
> everything shares the GlobalTimeStamp. When ITK is built with static
> libraries, every binary that links in libITKCommon will get its own
> GlobalTimeStamp.
>
> Unfortunately, a shared libITKCommon is not always possible. For the
> official Python packaging format, wheels, native libraries are not yet
> officially supported [1].
>
> The advantages of not linking everything into one, giant, monolithic
> `_ITK*Python.so` are that 1) we can lazily load only the parts of the
> library that we need, and 2) the brilliant ITK community members can
> extend ITK to create their own ITK Python module. For example, see the
> awesome ITKMinimalPathExtraction [2] or ITKIsotropicWavelets [3]
> modules.
>
> While I also investigated, for example, using CMake object libraries
> to make itk::TimeStamp an exported symbol in _ITKCommonPython.so and
> only linked there [4], this is a more complex build configuration.
> Additionally, according to the Python documentation, exporting extra
> symbols in a CPython extension module is officially not supported [5].
> In practice, I have found that extra symbols can segfault the process
> on module import with Python 3.5 on Windows. So, the sharing is done
> the officially supported way, via PyCapsule.
>
>
> > Additionally, I am wondering if cross-module `dynamic_cast` works
> correctly
> > in the current situation -- because the typeinfos in each static module
> .so
> > have different addresses [4]. (I'm not sure how to create a situation to
> > test this from Python)
>
> Yes, since the Python wrapping covers the entire toolkit, it is an
> excellent stress test for the dynamic_cast issue. When we developed
> the dynamic_cast patch, we used Python wrapping with static libraries
> and experimentally enabled hidden symbols by default [6]. Without the
> dynamic_cast fix, the Python tests failed, and with the fix, they
> pass. It turns out that we missed some mark-up on a few classes as
> indicated by warnings in the Python wrapping and non-wrapping test
> failures, but we are working on resolving those now.
>
> Thanks,
> Matt
>
>
> [1] https://mail.python.org/pipermail/wheel-builders/2016-April/
> 000090.html
>
> [2] https://github.com/InsightSoftwareConsortium/ITKMinimalPathExtraction
>
> [3] https://github.com/phcerdan/ITKIsotropicWavelets
>
> [4] https://github.com/thewtex/ITK/commits/thewtex-PythonModifiedTime55
>
> [5] https://docs.python.org/3.5/extending/extending.html#providi
> ng-a-c-api-for-an-extension-module
>
> [6] http://review.source.kitware.com/#/c/21766/
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://public.kitware.com/pipermail/insight-developers/attachments/20161222/e126f657/attachment.html>