Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve thread-safety, reduce code duplication internal I/O factory registration #2819

Merged

Conversation

N-Dekker
Copy link
Contributor

Improved thread-safety by replacing the use of file-scope static HasBeenRegistered variables with C++11 "magic statics", to implement internal I/O factory registration.

Reduced code duplication by adding a helper member function template, ObjectFactoryBase::RegisterInternalFactoryOnce<TFactory>().

@github-actions github-actions bot added area:Core Issues affecting the Core module area:IO Issues affecting the IO module area:Video Issues affecting the Video module labels Oct 15, 2021
Eases registering an internal factory exactly once, taking thread-safety
into account by means of C++11 "magic statics".
Replaced the use of file-scope static `HasBeenRegistered` variables for
conditional `RegisterOneFactory()` calls by the equivalent thread-safe
`ObjectFactoryBase::RegisterInternalFactoryOnce<TFactory>()` calls.

Using Notepad++ v8.1.4, Find in Files (Regular expression):

    Find what: if \(!.+IOFactoryHasBeenRegistered\)\r\n  {\r\n    .+IOFactoryHasBeenRegistered = true;\r\n    (.+)::RegisterOneFactory\(\);\r\n  }

    Replace with: ObjectFactoryBase::RegisterInternalFactoryOnce<$1>\(\);
@N-Dekker N-Dekker marked this pull request as ready for review October 15, 2021 15:51
Copy link
Contributor

@Leengit Leengit left a comment

Choose a reason for hiding this comment

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

Looks good to me.

Copy link
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.

@N-Dekker cool!

@thewtex thewtex requested a review from tbirdso October 20, 2021 19:43
Copy link
Contributor

@tbirdso tbirdso left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@thewtex thewtex merged commit da7f448 into InsightSoftwareConsortium:master Oct 21, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:Core Issues affecting the Core module area:IO Issues affecting the IO module area:Video Issues affecting the Video module
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants