[libcamera-devel] [PATCH v1] libcamera: framebuffer_allocator: Avoid double map lookup

[Kieran Bingham]

Quoting Laurent Pinchart via libcamera-devel (2022-11-28 06:37:56)
> Hi Barnabás,
> 
> Thank you for the patch.
> 
> On Sat, Nov 26, 2022 at 11:42:27PM +0000, Barnabás Pőcze via libcamera-devel wrote:
> > Use `try_emplace()` on the map instead of `count()`
> > and `operator[]` to avoid walking the tree twice.
> > 
> > Signed-off-by: Barnabás Pőcze <pobrn at protonmail.com>
> > ---
> >  src/libcamera/framebuffer_allocator.cpp | 6 ++++--
> >  1 file changed, 4 insertions(+), 2 deletions(-)
> > 
> > diff --git a/src/libcamera/framebuffer_allocator.cpp b/src/libcamera/framebuffer_allocator.cpp
> > index 4df27cac..17c5db49 100644
> > --- a/src/libcamera/framebuffer_allocator.cpp
> > +++ b/src/libcamera/framebuffer_allocator.cpp
> > @@ -88,12 +88,14 @@ FrameBufferAllocator::~FrameBufferAllocator()
> >   */
> >  int FrameBufferAllocator::allocate(Stream *stream)
> >  {
> > -     if (buffers_.count(stream)) {
> > +     const auto &[ it, inserted ] = buffers_.try_emplace(stream);
> 
> No spaces within [ ... ]
> 
> Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

LGTM too.


Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>

> 
> No need to resubmit if you agree with this change, I'll fix when
> applying.
> 
> > +
> > +     if (!inserted) {
> >               LOG(Allocator, Error) << "Buffers already allocated for stream";
> >               return -EBUSY;
> >       }
> > 
> > -     int ret = camera_->exportFrameBuffers(stream, &buffers_[stream]);
> > +     int ret = camera_->exportFrameBuffers(stream, &it->second);
> >       if (ret == -EINVAL)
> >               LOG(Allocator, Error)
> >                       << "Stream is not part of " << camera_->id()
> 
> -- 
> Regards,
> 
> Laurent Pinchart