{"id":22523,"url":"https://patchwork.libcamera.org/api/patches/22523/?format=json","web_url":"https://patchwork.libcamera.org/patch/22523/","project":{"id":1,"url":"https://patchwork.libcamera.org/api/projects/1/?format=json","name":"libcamera","link_name":"libcamera","list_id":"libcamera_core","list_email":"libcamera-devel@lists.libcamera.org","web_url":"","scm_url":"","webscm_url":""},"msgid":"<20250111114739.170891-1-pobrn@protonmail.com>","date":"2025-01-11T11:47:41","name":"[RFC,v1] pipeline: rkisp1: Avoid unnecessary indirection in `std::map`","commit_ref":null,"pull_url":null,"state":"superseded","archived":false,"hash":"d122d720c6c810c544b20fd8399801f0b02f6d88","submitter":{"id":133,"url":"https://patchwork.libcamera.org/api/people/133/?format=json","name":"Pőcze Barnabás","email":"pobrn@protonmail.com"},"delegate":null,"mbox":"https://patchwork.libcamera.org/patch/22523/mbox/","series":[{"id":4943,"url":"https://patchwork.libcamera.org/api/series/4943/?format=json","web_url":"https://patchwork.libcamera.org/project/libcamera/list/?series=4943","date":"2025-01-11T11:47:41","name":"[RFC,v1] pipeline: rkisp1: Avoid unnecessary indirection in `std::map`","version":1,"mbox":"https://patchwork.libcamera.org/series/4943/mbox/"}],"comments":"https://patchwork.libcamera.org/api/patches/22523/comments/","check":"pending","checks":"https://patchwork.libcamera.org/api/patches/22523/checks/","tags":{},"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 A6368C32F9\n\tfor ;\n\tSat, 11 Jan 2025 11:47:47 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id AD026684EA;\n\tSat, 11 Jan 2025 12:47:46 +0100 (CET)","from mail-4316.protonmail.ch (mail-4316.protonmail.ch\n\t[185.70.43.16])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 60723684D0\n\tfor ;\n\tSat, 11 Jan 2025 12:47:45 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=protonmail.com header.i=@protonmail.com\n\theader.b=\"xwsgmJTL\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=protonmail.com;\n\ts=protonmail3; t=1736596064; x=1736855264;\n\tbh=OF4S+iPihvh6yurMp6DRmIJDSMM0dkSg6BGEGdgufpI=;\n\th=Date:To:From:Subject:Message-ID:Feedback-ID:From:To:Cc:Date:\n\tSubject:Reply-To:Feedback-ID:Message-ID:BIMI-Selector:\n\tList-Unsubscribe:List-Unsubscribe-Post;\n\tb=xwsgmJTLz1JWXr9u0ow3UvGMDUF81f5Mz2vo7PW5wAD601sJXGcdqxHuzyzHkjBQx\n\tbb17Dy2voNpYaaL0YWIBzCaq2aevBdyswKA6I4O+jreoxvC3uqDHUS9oKQt2+y9902\n\t+2qymdv4AqVPzc1jvLE++MHWg/GGLxdjzGJHv1ucPjXLcRqCOrVxICuEIyudfiHqTt\n\thVYreU5eE5zJdONGiCDwz4s+DHvj5RC8cL+YZZcXUTfOHlpZplKJpdDasL3WXMWGEf\n\tjc9xlfVznl6HEbluOJOgo0c9VpYThcflXjUUHH7ugcQYah77n86QsF7n0Oo1e/NWf8\n\tl7UgmopwcPjSw==","Date":"Sat, 11 Jan 2025 11:47:41 +0000","To":"libcamera-devel@lists.libcamera.org","From":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Subject":"[RFC PATCH v1] pipeline: rkisp1: Avoid unnecessary indirection in\n\t`std::map`","Message-ID":"<20250111114739.170891-1-pobrn@protonmail.com>","Feedback-ID":"20568564:user:proton","X-Pm-Message-ID":"14f321d859b4d9909ef1315a646920b93b9a6ca0","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Transfer-Encoding":"quoted-printable","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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "},"content":"There is no reason to allocate the `RkISP1FrameInfo` objects\ndynamically, and then store raw pointers in the `std::map`.\n\nInstead, store the objects directly in the map. This removes\nthe need for manully calling new/delete, simplifies the code,\nand eliminates one memory allocation per frame.\n\nSigned-off-by: Barnabás Pőcze \n---\n src/libcamera/pipeline/rkisp1/rkisp1.cpp | 76 +++++++++++-------------\n 1 file changed, 34 insertions(+), 42 deletions(-)","diff":"diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\nindex 35c793da9..111acc35b 100644\n--- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n+++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n@@ -84,7 +84,7 @@ public:\n \n private:\n \tPipelineHandlerRkISP1 *pipe_;\n-\tstd::map frameInfo_;\n+\tstd::map frameInfo_;\n };\n \n class RkISP1CameraData : public Camera::Private\n@@ -269,49 +269,45 @@ RkISP1FrameInfo *RkISP1Frames::create(const RkISP1CameraData *data, Request *req\n \t\tmainPathBuffer = request->findBuffer(&data->mainPathStream_);\n \tselfPathBuffer = request->findBuffer(&data->selfPathStream_);\n \n-\tRkISP1FrameInfo *info = new RkISP1FrameInfo;\n+\tauto [it, inserted] = frameInfo_.try_emplace(frame);\n+\tASSERT(inserted);\n \n-\tinfo->frame = frame;\n-\tinfo->request = request;\n-\tinfo->paramBuffer = paramBuffer;\n-\tinfo->mainPathBuffer = mainPathBuffer;\n-\tinfo->selfPathBuffer = selfPathBuffer;\n-\tinfo->statBuffer = statBuffer;\n-\tinfo->paramDequeued = false;\n-\tinfo->metadataProcessed = false;\n+\tauto &info = it->second;\n \n-\tframeInfo_[frame] = info;\n+\tinfo.frame = frame;\n+\tinfo.request = request;\n+\tinfo.paramBuffer = paramBuffer;\n+\tinfo.mainPathBuffer = mainPathBuffer;\n+\tinfo.selfPathBuffer = selfPathBuffer;\n+\tinfo.statBuffer = statBuffer;\n+\tinfo.paramDequeued = false;\n+\tinfo.metadataProcessed = false;\n \n-\treturn info;\n+\treturn &info;\n }\n \n int RkISP1Frames::destroy(unsigned int frame)\n {\n-\tRkISP1FrameInfo *info = find(frame);\n-\tif (!info)\n+\tauto it = frameInfo_.find(frame);\n+\tif (it == frameInfo_.end())\n \t\treturn -ENOENT;\n \n-\tpipe_->availableParamBuffers_.push(info->paramBuffer);\n-\tpipe_->availableStatBuffers_.push(info->statBuffer);\n-\tpipe_->availableMainPathBuffers_.push(info->mainPathBuffer);\n-\n-\tframeInfo_.erase(info->frame);\n+\tauto &info = it->second;\n+\tpipe_->availableParamBuffers_.push(info.paramBuffer);\n+\tpipe_->availableStatBuffers_.push(info.statBuffer);\n+\tpipe_->availableMainPathBuffers_.push(info.mainPathBuffer);\n \n-\tdelete info;\n+\tframeInfo_.erase(it);\n \n \treturn 0;\n }\n \n void RkISP1Frames::clear()\n {\n-\tfor (const auto &entry : frameInfo_) {\n-\t\tRkISP1FrameInfo *info = entry.second;\n-\n-\t\tpipe_->availableParamBuffers_.push(info->paramBuffer);\n-\t\tpipe_->availableStatBuffers_.push(info->statBuffer);\n-\t\tpipe_->availableMainPathBuffers_.push(info->mainPathBuffer);\n-\n-\t\tdelete info;\n+\tfor (const auto &[frame, info] : frameInfo_) {\n+\t\tpipe_->availableParamBuffers_.push(info.paramBuffer);\n+\t\tpipe_->availableStatBuffers_.push(info.statBuffer);\n+\t\tpipe_->availableMainPathBuffers_.push(info.mainPathBuffer);\n \t}\n \n \tframeInfo_.clear();\n@@ -322,7 +318,7 @@ RkISP1FrameInfo *RkISP1Frames::find(unsigned int frame)\n \tauto itInfo = frameInfo_.find(frame);\n \n \tif (itInfo != frameInfo_.end())\n-\t\treturn itInfo->second;\n+\t\treturn &itInfo->second;\n \n \tLOG(RkISP1, Fatal) << \"Can't locate info from frame\";\n \n@@ -331,14 +327,12 @@ RkISP1FrameInfo *RkISP1Frames::find(unsigned int frame)\n \n RkISP1FrameInfo *RkISP1Frames::find(FrameBuffer *buffer)\n {\n-\tfor (auto &itInfo : frameInfo_) {\n-\t\tRkISP1FrameInfo *info = itInfo.second;\n-\n-\t\tif (info->paramBuffer == buffer ||\n-\t\t info->statBuffer == buffer ||\n-\t\t info->mainPathBuffer == buffer ||\n-\t\t info->selfPathBuffer == buffer)\n-\t\t\treturn info;\n+\tfor (auto &[frame, info] : frameInfo_) {\n+\t\tif (info.paramBuffer == buffer ||\n+\t\t info.statBuffer == buffer ||\n+\t\t info.mainPathBuffer == buffer ||\n+\t\t info.selfPathBuffer == buffer)\n+\t\t\treturn &info;\n \t}\n \n \tLOG(RkISP1, Fatal) << \"Can't locate info from buffer\";\n@@ -348,11 +342,9 @@ RkISP1FrameInfo *RkISP1Frames::find(FrameBuffer *buffer)\n \n RkISP1FrameInfo *RkISP1Frames::find(Request *request)\n {\n-\tfor (auto &itInfo : frameInfo_) {\n-\t\tRkISP1FrameInfo *info = itInfo.second;\n-\n-\t\tif (info->request == request)\n-\t\t\treturn info;\n+\tfor (auto &[frame, info] : frameInfo_) {\n+\t\tif (info.request == request)\n+\t\t\treturn &info;\n \t}\n \n \tLOG(RkISP1, Fatal) << \"Can't locate info from request\";\n","prefixes":["RFC","v1"]}