Skip to content

Global variable global synchronization#451

Closed
fbudin69500 wants to merge 1 commit intoInsightSoftwareConsortium:python-buildsfrom
hjmjohnson:global_variable_global_synchronization
Closed

Global variable global synchronization#451
fbudin69500 wants to merge 1 commit intoInsightSoftwareConsortium:python-buildsfrom
hjmjohnson:global_variable_global_synchronization

Conversation

@fbudin69500
Copy link
Copy Markdown
Contributor

@fbudin69500 fbudin69500 commented Jan 28, 2019

Replaces individual synchronization mechanisms across modules for each global
variables used in ITK by a central mechanism in which all these variables
are registered and which gives a single entry point to synchronize
all global variables.

Global variables are registered in a map, with a key that is their name and
a value that is a pair of their pointer and a pointer to a synchronization
function. The synchronization function will be called to synchronize
these global variables across the different modules. Different global
variables may have different synchronization functions: Some variables
will just override the previous value in all modules whereas some
others will need to actually synchronize their values.

Change-Id: Ib6ebc9b3f9acd93ce8319f00579f6bf4982ca6b8

@fbudin69500 fbudin69500 changed the title Global variable global synchronization WIP: Global variable global synchronization Jan 28, 2019
@fbudin69500
Copy link
Copy Markdown
Contributor Author

The problem on Windows could be the different conditions used on Windows, here:
https://github.com/InsightSoftwareConsortium/ITK/blob/master/Modules/Core/Common/src/CMakeLists.txt#L145-L147

and here:

https://github.com/InsightSoftwareConsortium/ITK/blob/master/Wrapping/Generators/Python/CMakeLists.txt#L433-L435

Although the two patches introducing the conditions are pretty old (8 months and 10 months). It is surprising that this has not caused an issue so far if this is indeed the root of the problem.

@thewtex thewtex requested a review from dzenanz January 29, 2019 15:17
Copy link
Copy Markdown
Member

@thewtex thewtex left a comment

Choose a reason for hiding this comment

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

Looking good!

A few inline comments.

Please rebase on master.

Comment thread Modules/Core/Common/include/itkBuildInformation.h Outdated
Comment thread Modules/Core/Common/include/itkSingleton.h Outdated
Comment thread Modules/Core/Common/include/itkSingleton.h Outdated
Comment thread Modules/Core/Common/include/itkSingletonMacro.h
Comment thread Modules/Core/Common/include/itkThreadPool.h Outdated
@fbudin69500 fbudin69500 force-pushed the global_variable_global_synchronization branch 5 times, most recently from 4ada86c to d62c059 Compare February 13, 2019 16:07
Replaces individual synchronization mechanisms across modules for each global
variables used in ITK by a central mechanism in which all these variables
are registered and which gives a single entry point to synchronize
all global variables.

Global variables are registered in a map, with a key that is their name and
a value that is a pair of their pointer and a pointer to a synchronization
function. The synchronization function will be called to synchronize
these global variables across the different modules. Different global
variables may have different synchronization functions: Some variables
will just override the previous value in all modules whereas some
others will need to actually synchronize their values.

Several macros defined in `itkSingletonMacro.h` facilitate the usage
of the new mechanism.

A few static global variables have yet to be integrated to this mechanism.
This included static variables defined in:
* itkTransformFactoryBase.h
* itkSpatialObjectFactoryBase.h
* itkFEMFactoryBase.h
* itkCurvatureNDAnisotropicDiffusionFunction.h
* itkLevelSetFunction.h

This list is not exhaustive and more global variables can be found with the
following command line in bash:

grep -r 'static '|grep m_ |grep -v ThirdParty |grep -v Review | grep -v const
@fbudin69500 fbudin69500 force-pushed the global_variable_global_synchronization branch from d62c059 to a66337e Compare February 13, 2019 16:11
@fbudin69500
Copy link
Copy Markdown
Contributor Author

On Windows, building ITK with MSVC++ 2017 with Python ON, and static libraries, the Python tests successfully pass.

@blowekamp
Copy link
Copy Markdown
Member

Are any additional tests needed?

@fbudin69500
Copy link
Copy Markdown
Contributor Author

Not to my knowledge, although anybody who wants to test this patch locally is more than welcome to do so and report the results here.

@thewtex thewtex changed the title WIP: Global variable global synchronization Global variable global synchronization Feb 15, 2019
@thewtex
Copy link
Copy Markdown
Member

thewtex commented Feb 15, 2019

Merged in #118

@thewtex thewtex closed this Feb 15, 2019
@hjmjohnson hjmjohnson deleted the global_variable_global_synchronization branch October 23, 2019 13:24
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.

3 participants