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

remove static lciostorer to allow multi-threading #62

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

fdplacido
Copy link

BEGINRELEASENOTES

  • Remove static singleton of LCIOStorer to be able to run various LCFIplus processors concurrently or in parallel.

ENDRELEASENOTES

@fdplacido
Copy link
Author

fdplacido commented Dec 3, 2021

With this, if two or more LCFIplus processors are used in the same event, LCIOStorer::SetEvent would be called more than once: clearing the objects and relations, and doing the conversions.

Instead of using a static object, or setting the event multiple times, one could also store the LCIOStorer object per event, and retrieve it in the following LCFIplus processors.

@suehara
Copy link
Member

suehara commented Dec 16, 2022

Sorry for not reviewing for very long time...

I don't know how multiple LCIOStorers work while the Event (and EventStore) is still a singleton.
Would you explain how you consider to run multi-threading? Processing a same event in parallel or treating completely different events at the same time?
If it's the first case maybe it's better to implement some signaling function to assure one event is loaded and all processors are finished before going to the next event (need some work). On the latter case everything needs to be duplicated, which may be possible but need careful consideration. At least Event class (and EventStore) should be kept one per thread and we need to ensure this.

@fdplacido
Copy link
Author

@andresailer

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.

2 participants