View Issue Details

IDProjectCategoryView StatusLast Update
0002813Slicer4Core: Base Codepublic2017-06-07 23:27
ReporterjcfrAssigned Tojcfr 
PriorityhighSeverityblockReproducibilityalways
Status closedResolutionfixed 
Product Version 
Target VersionSlicer 4.3.0Fixed in VersionSlicer 4.3.0 
Summary0002813: Restore performance of ITKv4 Factory
Description

See http://www.na-mic.org/Bug/view.php?id=2114#c7399

TagsITKv4

Relationships

related to 0002951 closedjcfr Update test framework to call initialize ITKIOFactoryRegistration when needed 
parent of 0002729 closedmccormic Recent ITK changes may have broken Slicer4/ITKv4 - Slicer can't seem to read volumes 
child of 0002114 closedmccormic Ensure Slicer can be compiled against ITKv4 on both Windows 32 and 64 bit 

Activities

hjmjohnson

hjmjohnson

2012-12-05 11:30

developer   ~0007414

Locations where factory registration needs to be optimized.

Brad L. <=====

$ find -name itkTransformIOFactoryRegisterManager.h
./Slicer-build/Libs/ITKFactoryRegistration/ITKIOFactoryRegistration/itkTransformIOFactoryRegisterManager.h
./CTK-build/CTK-build/Libs/ImageProcessing/ITK/Core/ITKIOFactoryRegistration/itkTransformIOFactoryRegisterManager.h
./SlicerExecutionModel-build/GenerateCLP/ITKIOFactoryRegistration/itkTransformIOFactoryRegisterManager.h
./SlicerExecutionModel-build/ModuleDescriptionParser/ITKIOFactoryRegistration/itkTransformIOFactoryRegisterManager.h

Hans

jcfr

jcfr

2012-12-05 14:06

administrator   ~0007415

Last edited: 2012-12-05 14:30

To optimize windows loading of images (and also potentially decrease the size of packages) the topic that should be tested on windows, macosx and linux are the following:
XXXX IGNORE - https://github.com/jcfr/Slicer/tree/ITKv4-shared-library-for-factory-initialization
XXXX IGNORE - https://github.com/jcfr/SlicerExecutionModel/tree/mechanism-to-link-executable-against-extra-libraries

=====
NOTE: See note below:

hjmjohnson

hjmjohnson

2012-12-05 14:32

developer   ~0007417

UPDATE:

Both patches from jcfr have been incorporated into a monolithic ITKv4 patch set.

git@github.com:BRAINSia/Slicer43.git

under the next-slicer43 branch

As of 2012-12-05, this was also in sync with the svn repository.

mccormic

mccormic

2012-12-06 12:18

developer   ~0007424

Hi Hans,

I did a fresh build with the BRAINSia/Slicer43/next-slicer43 branch with Visual Studio 2008 Express 64. However, it cannot load a file. In the "Log messages" there is a "Factories size: 3". Does this branch have a patch that still disables all factory registration?

Thanks,
Matt

jcfr

jcfr

2012-12-06 13:09

administrator   ~0007427

Hi Folks,

You should check with Brad Lowekamp, if I remember properly, the assumption was that simply linking Slicer against the shared library should be enough to ensure the default ITK IO Factory are loaded.

Seems this doesn't work as expected on windows. Would be nice to confirm it works on Linux/MacOSX. This the platform where Brad did his testing.

mccormic

mccormic

2012-12-06 14:34

developer   ~0007438

I have started a fresh Linux build. Will report the results.

jcfr

jcfr

2012-12-06 14:44

administrator   ~0007441

Sounds great. Thanks

mccormic

mccormic

2012-12-07 04:45

developer   ~0007445

Hmmm. Worked fine on Linux.

"Factories size: 21".

jcfr

jcfr

2012-12-07 05:16

administrator   ~0007447

Last edited: 2012-12-07 05:16

Just pushed something on my fork. If you try to apply the following patch on your windows checkout .. I am curious to see if the factory are loaded.

See https://github.com/jcfr/Slicer/commit/5b368c22c92a4005b84d0f69facb37bc55a185cd

To apply the patch:
save https://github.com/jcfr/Slicer/commit/5b368c22c92a4005b84d0f69facb37bc55a185cd.patch

Not I didn't tested it ...

apply using:
patch -p1 < 5b368c22c92a4005b84d0f69facb37bc55a185cd.patch

jcfr

jcfr

2013-01-07 08:56

administrator   ~0007625

Update the associated topic. See https://github.com/jcfr/Slicer/tree/ITKv4-shared-library-for-factory-initialization

jcfr

jcfr

2013-01-09 08:01

administrator   ~0007636

Topic has been updated with a commit allowing the factory to also be registered for CLI executable on Windows..

See https://github.com/jcfr/Slicer/commit/be6075a286d1a31aa3f9bee4abf154b82b223b1c

To test it, there are two options:
1) the option "Prefer CLI Executable" could be set in Slicer settings, then using a module like "GaussianBlur", the ITK factory should be loaded.
2) start a CLI from the command line using something like: C:\path\to\Slicer.exe --launch GaussianBlur.exe --param1 .. --param2 ...

jcfr

jcfr

2013-01-09 16:39

administrator   ~0007640

After integrating the previous slicer patch, you should also consider updating the SHA1 of the Slicer execution model to: https://github.com/jcfr/SlicerExecutionModel/commit/fded7173cdab39e399e6e0a53706c17b40d53c94

mccormic

mccormic

2013-01-09 18:56

developer   ~0007645

Great, thank you JC!

I have added the bump to 20130107-next-slicer43.

mccormic

mccormic

2013-02-12 07:33

developer   ~0007896

There are a number of IO related errors on the Windows build today (after the ITKv4 switch).

jcfr

jcfr

2013-02-12 07:40

administrator   ~0007897

The following patch should resolve that specific issue: https://github.com/jcfr/Slicer/commit/6a7effe2403fc4056808e1821878decd4340a629.patch

mccormic

mccormic

2013-02-12 07:52

developer   ~0007898

Well done!

jcfr

jcfr

2013-02-12 09:34

administrator   ~0007902

Should be fixed in r21691
See http://viewvc.slicer.org/viewvc.cgi/Slicer4?view=revision&revision=21691

jcfr

jcfr

2013-02-12 09:34

administrator   ~0007903

Please, re-open if there are still issue.

jcfr

jcfr

2013-02-16 09:46

administrator   ~0007968

// ------------------------------------
Commit r21721:

BUG: Ensure ITKv4 IO Factory are registered within tests

  • Include directories associated with ITKFactoryRegistration are also
    appended to ITK_INLCUDE_DIRS to ensure that the method itk::itkFactoryRegistration
    could be called.

  • Code allowing to register the factory has been manually added to the
    tests.

  • Would it make sens to have this library ITKFactoryIORegistration
    being built/provided by ITKv4 proper ?

See http://viewvc.slicer.org/viewvc.cgi/Slicer4?view=revision&revision=21721
// ------------------------------------

// ------------------------------------
Commit r21723:

COMP: Ensure ITKFactoryRegistration is considered when setting ITK_INCLUDE_DIRS at top-level.

This will fix build errors of the form:

.../Logic/Testing/Cxx/vtkMRMLSliceLogicTest2.cxx:49:38: error: itkFactoryRegistration.h: No such file or directory
.../Logic/Testing/Cxx/vtkMRMLSliceLogicTest2.cxx: In function 'int vtkMRMLSliceLogicTest2(int, char**)':
.../Logic/Testing/Cxx/vtkMRMLSliceLogicTest2.cxx:56: error: 'itkFactoryRegistration' is not a member of 'itk'

See http://viewvc.slicer.org/viewvc.cgi/Slicer4?view=revision&revision=21723
// ------------------------------------

// ------------------------------------
Commit r21725:

COMP: Fix missing "itkFactoryRegistrationConfigure.h" build error

This will fix build errors of the form:

../ITKFactoryRegistration/itkFactoryRegistration.h:6:45: error: itkFactoryRegistrationConfigure.h: No such file or directory
../Libs/MRML/Logic/Testing/Cxx/vtkMRMLSliceLogicTest2.cxx:49:
../Libs/ITKFactoryRegistration/itkFactoryRegistration.h:10: error: expected constructor, destructor, or type conversion before 'void'
../Libs/MRML/Logic/Testing/Cxx/vtkMRMLSliceLogicTest2.cxx: In function 'int vtkMRMLSliceLogicTest2(int, char**)':
../Libs/MRML/Logic/Testing/Cxx/vtkMRMLSliceLogicTest2.cxx:56: error: 'itkFactoryRegistration' is not a member of 'itk'

See http://viewvc.slicer.org/viewvc.cgi/Slicer4?view=revision&revision=21725
// ------------------------------------

jcfr

jcfr

2014-03-06 05:12

administrator   ~0011046

Closing resolved issues that have not been updated in more than 3 months

jcfr

jcfr

2017-06-07 23:27

administrator   ~0014670

Fix committed to 2145-support-for-installing-extension-from-file branch.

Related Changesets

Slicer: 2145-support-for-installing-extension-from-file 03b89614

2013-01-11 16:29:52

jcfr

Details Diff
COMP: Add ITKFactoryRegistration library centralizing ITK IO factory registration

This commit will ensure that ITK IO factory are properly registered on all
supported platforms.

When ITKv4 is build shared, the library holding the factory registration code
are build statically. As a consequence, when CLI module are loaded as library
the factory are registered multiple times. Around 800 factories where registered
and this was leading to poor performance when loading images.

This commit enable the building of a shared library named ITKFactoryRegistration
that should be linked against to ensure loading of the factory. This approach
can succeed thanks to the help of the ITK variable ITK_NO_IO_FACTORY_REGISTER_MANAGER.

This variable allow to disable the automatic registration of factory in selected part
of the code.

The following two cases are handled:
- Registration of the factories within the Slicer executable.
- Registration of the factories within CLI executable.

Fixes 0002813

From: Jean-Christophe Fillion-Robin <jchris.fillionr@kitware.com>

git-svn-id: http://svn.slicer.org/Slicer4/trunk@21592 3bd1e089-480b-0410-8dfb-8563597acbee
mod - Applications/SlicerApp/CMakeLists.txt Diff File
mod - Applications/SlicerApp/Main.cxx Diff File
mod - Base/CLI/CMakeLists.txt Diff File
add - Base/CLI/SEMCommandLineLibraryWrapper.cxx.in Diff File
mod - CMake/SlicerConfig.cmake.in Diff File
mod - CMake/SlicerGenerateSlicerConfig.cmake Diff File
mod - CMake/UseSlicer.cmake.in Diff File
mod - CMakeLists.txt Diff File
mod - Libs/CMakeLists.txt Diff File
mod - Libs/IGT/CMakeLists.txt Diff File
add - Libs/ITKFactoryRegistration/CMakeLists.txt Diff File
add - Libs/ITKFactoryRegistration/itkFactoryRegistration.cxx Diff File
add - Libs/ITKFactoryRegistration/itkFactoryRegistration.h Diff File
add - Libs/ITKFactoryRegistration/itkFactoryRegistrationConfigure.h.in Diff File
mod - Libs/MGHImageIO/CMakeLists.txt Diff File
mod - Libs/MRML/Core/CMakeLists.txt Diff File
mod - Libs/MRML/IDImageIO/CMakeLists.txt Diff File
mod - Libs/RemoteIO/CMakeLists.txt Diff File
mod - Libs/vtkITK/CMakeLists.txt Diff File
mod - Libs/vtkITK/Testing/CMakeLists.txt Diff File
mod - SuperBuild/External_SlicerExecutionModel.cmake Diff File

Issue History

Date Modified Username Field Change
2012-12-04 09:07 jcfr New Issue
2012-12-04 09:07 jcfr Status new => assigned
2012-12-04 09:07 jcfr Assigned To => pieper
2012-12-04 09:07 jcfr Relationship added child of 0002114
2012-12-04 09:08 jcfr Tag Attached: ITKv4
2012-12-04 09:08 jcfr Assigned To pieper => hjmjohnson
2012-12-04 09:08 jcfr Priority normal => high
2012-12-04 09:08 jcfr Severity minor => block
2012-12-04 09:08 jcfr Reproducibility have not tried => always
2012-12-04 09:08 jcfr Target Version => Slicer 4.3.0
2012-12-04 09:30 mccormic Assigned To hjmjohnson => mccormic
2012-12-05 11:30 hjmjohnson Note Added: 0007414
2012-12-05 14:06 jcfr Note Added: 0007415
2012-12-05 14:30 hjmjohnson Note Edited: 0007415
2012-12-05 14:32 hjmjohnson Note Added: 0007417
2012-12-06 12:18 mccormic Note Added: 0007424
2012-12-06 13:09 jcfr Note Added: 0007427
2012-12-06 14:34 mccormic Note Added: 0007438
2012-12-06 14:44 jcfr Note Added: 0007441
2012-12-07 04:45 mccormic Note Added: 0007445
2012-12-07 05:16 jcfr Note Added: 0007447
2012-12-07 05:16 jcfr Note Edited: 0007447
2012-12-08 05:20 jcfr Relationship added parent of 0002729
2013-01-07 08:56 jcfr Note Added: 0007625
2013-01-09 08:01 jcfr Note Added: 0007636
2013-01-09 16:39 jcfr Note Added: 0007640
2013-01-09 18:56 mccormic Note Added: 0007645
2013-02-12 07:33 mccormic Note Added: 0007896
2013-02-12 07:40 jcfr Note Added: 0007897
2013-02-12 07:52 mccormic Note Added: 0007898
2013-02-12 09:34 jcfr Note Added: 0007902
2013-02-12 09:34 jcfr Status assigned => resolved
2013-02-12 09:34 jcfr Fixed in Version => Slicer 4.3.0
2013-02-12 09:34 jcfr Resolution open => fixed
2013-02-12 09:34 jcfr Note Added: 0007903
2013-02-16 09:46 jcfr Note Added: 0007968
2013-02-16 09:47 jcfr Relationship added related to 0002951
2014-03-06 05:12 jcfr Note Added: 0011046
2014-03-06 05:13 jcfr Status resolved => closed
2017-06-07 23:27 jcfr Changeset attached => Slicer 2145-support-for-installing-extension-from-file 03b89614
2017-06-07 23:27 jcfr Note Added: 0014670
2017-06-07 23:27 jcfr Assigned To mccormic => jcfr