<div dir="ltr">Hi Isaiah,<div><br></div><div>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:</div><div><br></div><div>  <a href="http://review.source.kitware.com/#/c/21934/">http://review.source.kitware.com/#/c/21934/</a></div><div><br></div><div>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. </div><div><br></div><div>Thanks,</div><div>Matt</div></div><div class="gmail_extra"><br><div class="gmail_quote">On Thu, Dec 22, 2016 at 12:55 PM, Isaiah Norton <span dir="ltr"><<a href="mailto:isaiah.norton@gmail.com" target="_blank">isaiah.norton@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">Hi Matt,<div><br></div><div>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.</div><div><br></div><div>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.</div><div><div><br></div><div>Best,<br></div><div><div>Isaiah</div></div></div><div><div class="h5"><div><br></div><div class="gmail_extra"><br><div class="gmail_quote">On Sat, Dec 17, 2016 at 10:41 AM, Matt McCormick <span dir="ltr"><<a href="mailto:matt.mccormick@kitware.com" target="_blank">matt.mccormick@kitware.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Hi Isaiah,<br>
<br>
Here is some more information and background.<br>
<span class="m_-8066193386054341928gmail-m_5323048295798396464gmail-m_-1629425476312495232gmail-"><br>
<br>
> What is the rationale behind creating so many separate, statically-linked<br>
> Python modules? (78 `_ITK*Python.so` files on my machine). Creating a single<br>
> bundle instead should resolve the original issue, and might also reduce the<br>
> size of the final library [3].<br>
<br>
</span>For background, the GlobalTimeStamp, is a very important variable in<br>
ITK (and VTK). This is an atomic integer that gets incremented when<br>
any itk::Object (most classes in ITK) is "Modified", i.e. changed to<br>
note that it should be updated in a processing pipeline. So, almost<br>
all ITK-based code needs it, and it needs to be global across the<br>
entire program.<br>
<br>
The GlobalTimeStamp lives in and is used my itk::TimeStamp.cxx, and it<br>
is incremented on every call to ::Modified(). The itk::TimeStamp class<br>
is linked into libITKCommon.<br>
<br>
When ITK is built with shared libraries, libITKCommon is shared, and<br>
everything shares the GlobalTimeStamp. When ITK is built with static<br>
libraries, every binary that links in libITKCommon will get its own<br>
GlobalTimeStamp.<br>
<br>
Unfortunately, a shared libITKCommon is not always possible. For the<br>
official Python packaging format, wheels, native libraries are not yet<br>
officially supported [1].<br>
<br>
The advantages of not linking everything into one, giant, monolithic<br>
`_ITK*Python.so` are that 1) we can lazily load only the parts of the<br>
library that we need, and 2) the brilliant ITK community members can<br>
extend ITK to create their own ITK Python module. For example, see the<br>
awesome ITKMinimalPathExtraction [2] or ITKIsotropicWavelets [3]<br>
modules.<br>
<br>
While I also investigated, for example, using CMake object libraries<br>
to make itk::TimeStamp an exported symbol in _ITKCommonPython.so and<br>
only linked there [4], this is a more complex build configuration.<br>
Additionally, according to the Python documentation, exporting extra<br>
symbols in a CPython extension module is officially not supported [5].<br>
In practice, I have found that extra symbols can segfault the process<br>
on module import with Python 3.5 on Windows. So, the sharing is done<br>
the officially supported way, via PyCapsule.<br>
<span class="m_-8066193386054341928gmail-m_5323048295798396464gmail-m_-1629425476312495232gmail-"><br>
<br>
> Additionally, I am wondering if cross-module `dynamic_cast` works correctly<br>
> in the current situation -- because the typeinfos in each static module .so<br>
> have different addresses [4]. (I'm not sure how to create a situation to<br>
> test this from Python)<br>
<br>
</span>Yes, since the Python wrapping covers the entire toolkit, it is an<br>
excellent stress test for the dynamic_cast issue. When we developed<br>
the dynamic_cast patch, we used Python wrapping with static libraries<br>
and experimentally enabled hidden symbols by default [6]. Without the<br>
dynamic_cast fix, the Python tests failed, and with the fix, they<br>
pass. It turns out that we missed some mark-up on a few classes as<br>
indicated by warnings in the Python wrapping and non-wrapping test<br>
failures, but we are working on resolving those now.<br>
<br>
Thanks,<br>
Matt<br>
<br>
<br>
[1] <a href="https://mail.python.org/pipermail/wheel-builders/2016-April/000090.html" rel="noreferrer" target="_blank">https://mail.python.org/piperm<wbr>ail/wheel-builders/2016-April/<wbr>000090.html</a><br>
<br>
[2] <a href="https://github.com/InsightSoftwareConsortium/ITKMinimalPathExtraction" rel="noreferrer" target="_blank">https://github.com/InsightSoft<wbr>wareConsortium/ITKMinimalPathE<wbr>xtraction</a><br>
<br>
[3] <a href="https://github.com/phcerdan/ITKIsotropicWavelets" rel="noreferrer" target="_blank">https://github.com/phcerdan/IT<wbr>KIsotropicWavelets</a><br>
<br>
[4] <a href="https://github.com/thewtex/ITK/commits/thewtex-PythonModifiedTime55" rel="noreferrer" target="_blank">https://github.com/thewtex/ITK<wbr>/commits/thewtex-PythonModifie<wbr>dTime55</a><br>
<br>
[5] <a href="https://docs.python.org/3.5/extending/extending.html#providing-a-c-api-for-an-extension-module" rel="noreferrer" target="_blank">https://docs.python.org/3.5/ex<wbr>tending/extending.html#providi<wbr>ng-a-c-api-for-an-extension-mo<wbr>dule</a><br>
<br>
[6] <a href="http://review.source.kitware.com/#/c/21766/" rel="noreferrer" target="_blank">http://review.source.kitware.c<wbr>om/#/c/21766/</a><br>
</blockquote></div><br></div></div></div></div>
</blockquote></div><br></div>