[RFC] Modernizing product retrieval from Event

[makortel]

Currently products can be retrieved from edm::Event with

edm::EDGetTokenT<Foo> token_;
...
// always access, throws if product is not available
edm::Handle<Foo> handle;
iEvent.getByToken(token_, handle);
auto const& foo = *handle;

// free function to simplify the above
auto const& foo = edm::get(iEvent, token_);

// check product availability with the Handle
edm::Handle<Foo> handle;
iEvent.getByToken(token_, handle);
if(handle.isValid()) {
  auto const& foo = *handle;
}

// check product availability with the getByToken return value
edm::Handle<Foo> handle;
if(iEvent.getByToken(token_, handle)) {
  auto const& foo = *handle;
}

In above the token_ can also be the untyped edm::EDGetToken (or edm::InputTag with getByLabel()).

These retrieval calls could be modernized by introducing edm::ValidHandle<T>, which would check the existence of the product in its constructor. Compared to edm::Handle, it would be faster to use as its accessors would not need any further checks. Compared to const reference, it would still provide access to the provenance. It would be accompanied with the following interface

edm::EDGetTokenT<Foo> token_;
...
// Returns edm::Handle<Foo>
auto handle = iEvent.getHandle(token_);
auto const& foo = *handle;

// edm::ValidHandle from implicit conversion
edm::ValidHandle<Foo> handle = iEvent.getHandle(token_);
// edm::ValidHandle from explicit conversion
auto handle = iEvent.getHandle(token_).makeValidHandle();

// const reference like with the free get()
// throws if product does not exist
auto const& foo = iEvent.get(token_);

// const pointer that allows checking whether the product exists or not
// does not throw if the product does not exist
if(auto const* fooPtr = iEvent.tryToGet(token_)) {
  ...
}

These interfaces would support only the typed edm::EDGetTokenT<T>.

[cmsbuild]

A new Issue was created by @makortel Matti Kortelainen.

@davidlange6, @Dr15Jones, @smuzaffar, @fabiocos, @kpedro88 can you please review it and eventually sign/assign? Thanks.

cms-bot commands are listed here

[guitargeek]

I love that "issue", that interface would be a great update to the framework!

What would you think of shortening iEvent.getHandle(token_).makeValidHandle() to iEvent.getValidHandle(token_)? Then I can at least say I was foreshadowing the new interface with my weird MultiToken class [1] :D

[1] https://github.com/cms-sw/cmssw/blob/master/RecoEgamma/EgammaTools/interface/MultiToken.h#L95

[makortel]

We should add to the contract of Event::get() that the caller is not permitted to store pointers/references to the returned product (or to anything it contains) that would live beyond the end of the analyze()/produce()/filter() function. Then this piece of information could be used in improving "early deletion" of event products (#16481).

[Dr15Jones]

::get and ::getHandle are available here: #25707

The use of a edm::ValidHandle was changed to have the syntax

auto handle = makeValid(iEvent.getHandle(token_));

as this avoid coupling edm::ValidHandle to edm::Handle as well as allows conversion for any Handle like class (e.g. edm::OrphanHandle).

[Dr15Jones]

assign core

[cmsbuild]

New categories assigned: core

@Dr15Jones,@smuzaffar you have been requested to review this Pull request/Issue and eventually sign? Thanks

[makortel]

We should add to the contract of Event::get() that the caller is not permitted to store pointers/references to the returned product (or to anything it contains) that would live beyond the end of the analyze()/produce()/filter() function. Then this piece of information could be used in improving "early deletion" of event products (#16481).

With mechanism like #16481 (comment) the restriction above would not be needed.

[Dr15Jones]

+1

[cmsbuild]

This issue is fully signed and ready to be closed.