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

[Matt McCormick]

Hi Isaiah,

Yes, good point. Furthermore, if there is any other binary (non-Python
extension), they will also run into the timestamp / unique singleton issue.
To catch all cases, we can suggest that whenever using multiple binaries
that link to ITK, shared libraries should be used. Please review this
addition to the documentation:

  http://review.source.kitware.com/#/c/21934/

Since we don't know how folks will be using ITK when they are configuring
ITK, it is difficult to provide a warning there that is not confusing. I
think that if the developer is sophisticated enough to embed Python and
pass objects into that embedded Python, they will be using shared
libraries. The default static libraries is the easiest configuration for
beginners building stand-alone executables or Python Windows users that do
not want / need to tweak the PATH to find .dlls.

Thanks,
Matt

On Thu, Dec 22, 2016 at 12:55 PM, Isaiah Norton <isaiah.norton at gmail.com>
wrote:

> 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/20170109/61588b2e/attachment.html>