[{"id":25914,"web_url":"https://patchwork.libcamera.org/comment/25914/","msgid":"","date":"2022-11-28T06:37:56","subject":"Re: [libcamera-devel] [PATCH v1] libcamera: framebuffer_allocator:\n\tAvoid double map lookup","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Barnabás,\n\nThank you for the patch.\n\nOn Sat, Nov 26, 2022 at 11:42:27PM +0000, Barnabás Pőcze via libcamera-devel wrote:\n> Use `try_emplace()` on the map instead of `count()`\n> and `operator[]` to avoid walking the tree twice.\n> \n> Signed-off-by: Barnabás Pőcze \n> ---\n> src/libcamera/framebuffer_allocator.cpp | 6 ++++--\n> 1 file changed, 4 insertions(+), 2 deletions(-)\n> \n> diff --git a/src/libcamera/framebuffer_allocator.cpp b/src/libcamera/framebuffer_allocator.cpp\n> index 4df27cac..17c5db49 100644\n> --- a/src/libcamera/framebuffer_allocator.cpp\n> +++ b/src/libcamera/framebuffer_allocator.cpp\n> @@ -88,12 +88,14 @@ FrameBufferAllocator::~FrameBufferAllocator()\n> */\n> int FrameBufferAllocator::allocate(Stream *stream)\n> {\n> -\tif (buffers_.count(stream)) {\n> +\tconst auto &[ it, inserted ] = buffers_.try_emplace(stream);\n\nNo spaces within [ ... ]\n\nReviewed-by: Laurent Pinchart \n\nNo need to resubmit if you agree with this change, I'll fix when\napplying.\n\n> +\n> +\tif (!inserted) {\n> \t\tLOG(Allocator, Error) << \"Buffers already allocated for stream\";\n> \t\treturn -EBUSY;\n> \t}\n> \n> -\tint ret = camera_->exportFrameBuffers(stream, &buffers_[stream]);\n> +\tint ret = camera_->exportFrameBuffers(stream, &it->second);\n> \tif (ret == -EINVAL)\n> \t\tLOG(Allocator, Error)\n> \t\t\t<< \"Stream is not part of \" << camera_->id()","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 90922BE08B\n\tfor ;\n\tMon, 28 Nov 2022 06:38:13 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id CCF3D63335;\n\tMon, 28 Nov 2022 07:38:12 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 6F42661F24\n\tfor ;\n\tMon, 28 Nov 2022 07:38:11 +0100 (CET)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id D7112501;\n\tMon, 28 Nov 2022 07:38:10 +0100 (CET)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1669617492;\n\tbh=Rbc5q9En4x9U91E5GtlBpk/Wu5E5aaXYFFRfH9VYPS4=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=s0R3p2m+2Zb8BWILZ8qKM/MZV22BeCh5asZQcw5ICY9VOBJy2ZmURSFsxtqlJR+bW\n\tSSgcOptYgJXxmrvIhiYE28cflYfAtCSWKVN2FH6r+WI8Cv5uXflZ9yWT3LwdYxBnOH\n\tzMWO1/eXwTHVTRIYaTApNiQgLHkYIeEnNJJZXv8kZnfK87Cvi8cfGHThyg/cAZ/xpI\n\t34cjoWBrsl47TJlmPXJhIGn0DK/qiJVMe/TDRhjeAggtU03yHFBfzoNnT96S1yrtxz\n\trKkWTXBI69PtiPG9nILy+1sUhHlOcmKwm1OsIhJ1+XDnZ9Q3bONyqBWSIfbtnEqiBd\n\tDNYbj3Fcac3RQ==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1669617491;\n\tbh=Rbc5q9En4x9U91E5GtlBpk/Wu5E5aaXYFFRfH9VYPS4=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=NM+RMSiHCHyP/pC1/Xi+c+Gg+CDSsTrcK6oGMHXDo5Tjh4uN22KPkTSaH24Eltek0\n\t852dd661K/akL+ofZWEQMo0UhOH3PgohZoTJYlYjp3JdhDxJfyRCEpbJhrY8J9+t3p\n\tVm9j9I3b0n/mXAxvHs+vwWTYC4+XA1Pc67VM9IZQ="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"NM+RMSiH\"; dkim-atps=neutral","Date":"Mon, 28 Nov 2022 08:37:56 +0200","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Message-ID":"","References":"","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"","Subject":"Re: [libcamera-devel] [PATCH v1] libcamera: framebuffer_allocator:\n\tAvoid double map lookup","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","From":"Laurent Pinchart via libcamera-devel\n\t","Reply-To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":25915,"web_url":"https://patchwork.libcamera.org/comment/25915/","msgid":"<166962834880.1079859.15371674425070614849@Monstersaurus>","date":"2022-11-28T09:39:08","subject":"Re: [libcamera-devel] [PATCH v1] libcamera: framebuffer_allocator:\n\tAvoid double map lookup","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Laurent Pinchart via libcamera-devel (2022-11-28 06:37:56)\n> Hi Barnabás,\n> \n> Thank you for the patch.\n> \n> On Sat, Nov 26, 2022 at 11:42:27PM +0000, Barnabás Pőcze via libcamera-devel wrote:\n> > Use `try_emplace()` on the map instead of `count()`\n> > and `operator[]` to avoid walking the tree twice.\n> > \n> > Signed-off-by: Barnabás Pőcze \n> > ---\n> > src/libcamera/framebuffer_allocator.cpp | 6 ++++--\n> > 1 file changed, 4 insertions(+), 2 deletions(-)\n> > \n> > diff --git a/src/libcamera/framebuffer_allocator.cpp b/src/libcamera/framebuffer_allocator.cpp\n> > index 4df27cac..17c5db49 100644\n> > --- a/src/libcamera/framebuffer_allocator.cpp\n> > +++ b/src/libcamera/framebuffer_allocator.cpp\n> > @@ -88,12 +88,14 @@ FrameBufferAllocator::~FrameBufferAllocator()\n> > */\n> > int FrameBufferAllocator::allocate(Stream *stream)\n> > {\n> > - if (buffers_.count(stream)) {\n> > + const auto &[ it, inserted ] = buffers_.try_emplace(stream);\n> \n> No spaces within [ ... ]\n> \n> Reviewed-by: Laurent Pinchart \n\nLGTM too.\n\n\nReviewed-by: Kieran Bingham \n\n> \n> No need to resubmit if you agree with this change, I'll fix when\n> applying.\n> \n> > +\n> > + if (!inserted) {\n> > LOG(Allocator, Error) << \"Buffers already allocated for stream\";\n> > return -EBUSY;\n> > }\n> > \n> > - int ret = camera_->exportFrameBuffers(stream, &buffers_[stream]);\n> > + int ret = camera_->exportFrameBuffers(stream, &it->second);\n> > if (ret == -EINVAL)\n> > LOG(Allocator, Error)\n> > << \"Stream is not part of \" << camera_->id()\n> \n> -- \n> Regards,\n> \n> Laurent Pinchart","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 2CC1EBDE6B\n\tfor ;\n\tMon, 28 Nov 2022 09:39:14 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 6505D6332B;\n\tMon, 28 Nov 2022 10:39:13 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id CE16D63316\n\tfor ;\n\tMon, 28 Nov 2022 10:39:11 +0100 (CET)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust3082.18-1.cable.virginm.net [86.31.172.11])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 4DE0F501;\n\tMon, 28 Nov 2022 10:39:11 +0100 (CET)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1669628353;\n\tbh=EDoUykm4rXKmkQanyWC4Re54QwnfHO5LrW6mml8fF8s=;\n\th=In-Reply-To:References:To:Date:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=MZoQp1qyxoWxu+xweldciwjtohIXelBafuMJ0IBoQWxuYIFGxdCeEvSAtRT+6fzXm\n\tw4gFktgGPFdpze+EOcZIDfiLPZ8zDPl6jxJMaLFoCd8QZggMGygdAbPcYUgOwGkVgV\n\tGUH0Xuau0Des2RThAVQKnPZP+WRfcwA2Er0G4XFBQRLM0HGHE6kSOV4yYaOdTYm8It\n\tWiLleHnYao+xtwKx/+kRslS8wHNZD+9yVMKkjz65ed+7q3FpXCoJKW5DJ5olY6EV8E\n\tcnZ3UwVHQppnPUuZLSmkxuTbeGltBvp3vb+2dfQOuF7HAxrYmyoqAdwagt4ht9yymO\n\t+h/3XIYp6uqRA==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1669628351;\n\tbh=EDoUykm4rXKmkQanyWC4Re54QwnfHO5LrW6mml8fF8s=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=MckWCLVm84KcBMwg234exf2rgz/EfcAZMe66WcR3U1VIaH0BS+0dHCp6OwdTwF3Vc\n\tUgmMh98OUE+o+mNFzuIK1BjOgjj+RwJFW6IvuxswZWg2OXlgJ8eaFj13Rm8DIucPLx\n\tbcQOrdqDFFalA8WiWtyjQhkSAfzWBHyo6FMfGxt0="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"MckWCLVm\"; dkim-atps=neutral","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"","References":"\n\t","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ,\n\tLaurent Pinchart , \n\tLaurent Pinchart via\n\tlibcamera-devel ","Date":"Mon, 28 Nov 2022 09:39:08 +0000","Message-ID":"<166962834880.1079859.15371674425070614849@Monstersaurus>","User-Agent":"alot/0.10","Subject":"Re: [libcamera-devel] [PATCH v1] libcamera: framebuffer_allocator:\n\tAvoid double map lookup","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","From":"Kieran Bingham via libcamera-devel\n\t","Reply-To":"Kieran Bingham ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":25917,"web_url":"https://patchwork.libcamera.org/comment/25917/","msgid":"","date":"2022-11-28T13:59:55","subject":"Re: [libcamera-devel] [PATCH v1] libcamera: framebuffer_allocator:\n\tAvoid double map lookup","submitter":{"id":133,"url":"https://patchwork.libcamera.org/api/people/133/","name":"Pőcze Barnabás","email":"pobrn@protonmail.com"},"content":"2022. november 28., hétfő 7:37 keltezéssel, Laurent Pinchart írta:\n\n\n> Hi Barnabás,\n> \n> Thank you for the patch.\n> \n> On Sat, Nov 26, 2022 at 11:42:27PM +0000, Barnabás Pőcze via libcamera-devel wrote:\n> \n> > Use `try_emplace()` on the map instead of `count()`\n> > and `operator[]` to avoid walking the tree twice.\n> > \n> > Signed-off-by: Barnabás Pőcze pobrn@protonmail.com\n> > ---\n> > src/libcamera/framebuffer_allocator.cpp | 6 ++++--\n> > 1 file changed, 4 insertions(+), 2 deletions(-)\n> > \n> > diff --git a/src/libcamera/framebuffer_allocator.cpp b/src/libcamera/framebuffer_allocator.cpp\n> > index 4df27cac..17c5db49 100644\n> > --- a/src/libcamera/framebuffer_allocator.cpp\n> > +++ b/src/libcamera/framebuffer_allocator.cpp\n> > @@ -88,12 +88,14 @@ FrameBufferAllocator::~FrameBufferAllocator()\n> > */\n> > int FrameBufferAllocator::allocate(Stream *stream)\n> > {\n> > - if (buffers_.count(stream)) {\n> > + const auto &[ it, inserted ] = buffers_.try_emplace(stream);\n> \n> \n> No spaces within [ ... ]\n> \n> Reviewed-by: Laurent Pinchart laurent.pinchart@ideasonboard.com\n> \n> \n> No need to resubmit if you agree with this change, I'll fix when\n> applying.\n\nYes, please, fix it when applying.\n\n\nRegards,\nBarnabás Pőcze\n\n> \n> > +\n> > + if (!inserted) {\n> > LOG(Allocator, Error) << \"Buffers already allocated for stream\";\n> > return -EBUSY;\n> > }\n> > \n> > - int ret = camera_->exportFrameBuffers(stream, &buffers_[stream]);\n> > + int ret = camera_->exportFrameBuffers(stream, &it->second);\n> > if (ret == -EINVAL)\n> > LOG(Allocator, Error)\n> > << \"Stream is not part of \" << camera_->id()\n> \n> \n> --\n> Regards,\n> \n> Laurent Pinchart","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id D1E49BE08B\n\tfor ;\n\tMon, 28 Nov 2022 14:00:04 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 112E963335;\n\tMon, 28 Nov 2022 15:00:04 +0100 (CET)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1669644004;\n\tbh=SFLnzX+BvaRS3A5PfbDX14JSjkcQLNMpHGsvHpSD6TA=;\n\th=Date:To:In-Reply-To:References:List-Id:List-Post:From:Cc:\n\tList-Subscribe:List-Unsubscribe:List-Archive:Reply-To:List-Help:\n\tSubject:From;\n\tb=JmOvyaMXIiwVjtnVmWUpk6q3BFB4vlhJ69Ak1HxJiUXCp47MYPiicDTPOOxEp7CgP\n\tq6S6Cb57otKM2BeOc/M+zMS12IPIZ+tS35XpBj3RjEhGSVwWckCGX0rZbDf04Dng4F\n\tAcYWi0KnyHCVbeTE0a7l5G94+27HRQ9pbkczOmsVKUDXPeTOZXmRLY5cR0jp+4CU/S\n\tubM3W7vw9soQEWKJg/pyvM6YcajRBs6oYxVAXO8oYcCedBszYtTm9Wzt+8WftXwmHi\n\ti1cRlhhAPQzFOIEnwslKtfWJVRw3UVimt2WIwAfsSEJUFpGlrfpjM5jBj9337QVZ+o\n\tES11P+2OkwhMg==","Date":"Mon, 28 Nov 2022 13:59:55 +0000","To":"Laurent Pinchart ","In-Reply-To":"","References":"\n\t","MIME-Version":"1.0","Message-ID":"","List-Id":"","List-Post":"","From":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze_via_libcamera-devel?=\n\t","Precedence":"list","Cc":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","X-BeenThere":"libcamera-devel@lists.libcamera.org","List-Subscribe":",\n\t","List-Unsubscribe":",\n\t","List-Archive":"","Reply-To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","List-Help":"","Subject":"Re: [libcamera-devel] [PATCH v1] libcamera: framebuffer_allocator:\n\tAvoid double map lookup","Content-Type":"message/rfc822","Content-Disposition":"inline","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]