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

Post-processing for JEventSource::GetEvent #126

Open
faustus123 opened this issue Jul 30, 2022 · 5 comments
Open

Post-processing for JEventSource::GetEvent #126

faustus123 opened this issue Jul 30, 2022 · 5 comments

Comments

@faustus123
Copy link
Collaborator

A common scenario for event sources will be to read in the event and then process the data to instantiate objects from it. This second part can, in principle, be done in parallel. Right now, GetEvent is called sequentially using a mutex lock in JEventSourceDoNext. There may be a lot of ways to handle this (e.g. add another virtual method in addition to GetEvent called something like "PostProcessEvent" that gets called in parallel).

This is not an urgent issue.

@nathanwbrei
Copy link
Collaborator

We might be able to just add a flag to specify whether GetEvent is called sequentially or not. Having GetEvent be called in parallel feels much cleaner to me because GetEvent usually gets called from a parallel arrow.

@faustus123
Copy link
Collaborator Author

The only issue there is that in pretty much all cases, they will need to do at least some part of the I/O sequentially. Thus, the class would need to mark itself as parallel, then lock its own mutex while doing the sequential part and then unlock it to do the parallel part. This would give ultimate flexibility, but it might be nice to give an option so they don't have to handle the mutex. (I also realize we may be going a little overboard to shield the "fragile" developer from having to use a mutex when in reality, they should know how to deal with this stuff in modern computing environments!)

I guess I should note that the GetObjects method was intended to handle this. The issue there (as you may recall) is that we typically want to do a single pass over the input buffer and extract all object types. GetObjects was aimed at a model where one could random access the part of the event with the requested objects and quickly pull them out. That type of event source has never really been used with JANA.

So, yes, please add the flag so a user can specify that GetEvent be called in parallel. I would say we should also add the PostProcessEvent virtual method (named whatever you want). This would give them the option of doing it all without messing with their own mutex.

@nathanwbrei
Copy link
Collaborator

nathanwbrei commented Aug 16, 2022

I just realized I wrote "GetEvent" in my comment above when I meant "GetObjects", but I think we agree!

Both GetEvent() and GetObjects need to be optionally protected by a mutex, and that mutex should be controlled via a flag rather than directly by the user. (This has been a longstanding request, see issue #72). If the flag is set, the event source's arrow needs to be sequential; otherwise it can be parallel. I'm pretty sure we need to use the same mutex for both, but I think we might want separate flags to control whether GetEvent and GetObjects actually obtain the lock?

Meanwhile, PostProcessEvent() would always be run in parallel (otherwise the logic would simply go in GetEvent) and would work by calling JEvent->Insert() just like GetEvent(), and not do the antipatterny thing that GetObjects does.

The only thing I don't like about the name PostProcessEvent is that we also have FinishEvent, which is called after all JEventProcessors have run and the event is about to be reset and recycled. Is it intuitive to you that postprocessing happens before finishing? I'd like to propose the name "preprocess", because you can't preprocess an event until you get it, and the "preprocess" would run before the JEventProcessors' process methods run.

So the lifecycle of a JEvent would look like:

JEventPool::Get()
JEventSource::Get(event). (calls event->Insert)
JEventSource::Preprocess(event) (calls event->Insert)
JEventProcessor::Process(event) (potentially calls JEventSource::GetObjects(factory))
JEventSource::FinishEvent(event)
JEventPool::Put() (calls event->Reset)

@nathanwbrei
Copy link
Collaborator

Summary from our discussion:

  1. Control whether GetEvent() is sequential or parallel using a flag
  2. Have GetObjects() and PreprocessEvent() always be parallel

@faustus123
Copy link
Collaborator Author

that was my understanding as well

@nathanwbrei nathanwbrei added this to the v2.0.8: Support for EICrecon milestone Aug 18, 2022
@nathanwbrei nathanwbrei removed this from the v2.0.8: Support for EICrecon milestone Sep 22, 2023
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

No branches or pull requests

2 participants