From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from NAM10-DM6-obe.outbound.protection.outlook.com (mail-dm6nam10on2083.outbound.protection.outlook.com [40.107.93.83]) by sourceware.org (Postfix) with ESMTPS id E72963858C62 for ; Tue, 8 Nov 2022 19:48:06 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org E72963858C62 Authentication-Results: sourceware.org; dmarc=fail (p=quarantine dis=none) header.from=amd.com Authentication-Results: sourceware.org; spf=fail smtp.mailfrom=amd.com ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=EAakQ2OZ4NjQBU72sl1/QtWk41Y3+8NrWufX2kfrO3Fq8Bwbs0VU/v7/RLuP4UhN1XU1MKZIRopKSoCNBaYjcF2ND6E2kIi+RuQurPHdREfaVqQWZKwB+HqEbTHkTyESAbyzmyONOyVDYCSQQYjhwN8cicswpjTmEta1wzRJ7rVMkrYB5ZgLAy4n7UjlSylM9OP+lkYGpAe3+W6J/ftt58tAs4p5X0G/nco0nBK8gkwB7P2buSEYSARwZsLk0JoqcdMqpowLAho4+WF9ENfYrKE8yW7GbHIaYtiig3mRsJBSfJjWVgWmA2z3/yZiU0cI5+J1FHzdYlgy3H+OJAHNAQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=rDKTbRjig6FByUX6mnJwur/Q5pr9j6/U3YX6rYk63UA=; b=ipAfi9KTGJqBRtk1+Dv6wwxEq/Miq+PfV4uSXfB8EoSyqjpE35bxT4lmPtXp6gGuZNu1e1vkBvhou+T5vgEaQw+IirBrX3LS/fqvAA1KVMPs3KgMwe2bA8qEXFGa1LLvW/sYPzzGcVAVAeKALYuv+PmdIOWpzijEPCsYkLtGDPj4gx1Aq7BMuk6pX7srbdkK7V2cXMnZJpzgsxz4//4VcTvPPMoq9f4z0Hjjrmip/73YBhEhkJmpX5ifWUKn+5NDcghEOD1OqzY+iZJ64IGiRi0JkDRl0fq4iFl7fBZept6bwPbO01DK7fnUCNNWZBDf5am4/Md22Dszn5wrMfkXug== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=amd.com; dmarc=pass action=none header.from=amd.com; dkim=pass header.d=amd.com; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=amd.com; s=selector1; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=rDKTbRjig6FByUX6mnJwur/Q5pr9j6/U3YX6rYk63UA=; b=Wig1OZ+1pysAXNkR5KXpR1amUgUox83TWVD55QhiC3SRAbF1cSSsxbqsoXq2HgH/+R1BaZllSV+FBuHAM0wCTkQ6CcLlPqvblyHj0pC4ZKKwHBtrnwsQ/itaLdFv1OUi//qIAM3KyC/GYalM9AU8zyCwGdxHobSZZrN1a0/H0sI= Authentication-Results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=amd.com; Received: from DM4PR12MB5745.namprd12.prod.outlook.com (2603:10b6:8:5c::7) by DS0PR12MB7583.namprd12.prod.outlook.com (2603:10b6:8:13f::8) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.5791.22; Tue, 8 Nov 2022 19:48:03 +0000 Received: from DM4PR12MB5745.namprd12.prod.outlook.com ([fe80::15e7:2ef7:ccc0:7f52]) by DM4PR12MB5745.namprd12.prod.outlook.com ([fe80::15e7:2ef7:ccc0:7f52%6]) with mapi id 15.20.5791.024; Tue, 8 Nov 2022 19:48:03 +0000 Message-ID: <1fb8958a-a542-d989-9f0f-355340b19555@amd.com> Date: Tue, 8 Nov 2022 19:47:58 +0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.2.2 Subject: Re: [PATCH] gdb/py-inferior: Keep inferior threads in a map To: Tom Tromey , Lancelot SIX via Gdb-patches Cc: lsix@lancelotsix.com References: <20221107184727.2228056-1-lancelot.six@amd.com> <87o7th48p7.fsf@tromey.com> Content-Language: en-US From: Lancelot SIX In-Reply-To: <87o7th48p7.fsf@tromey.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-ClientProxiedBy: LO4P123CA0279.GBRP123.PROD.OUTLOOK.COM (2603:10a6:600:195::14) To DM4PR12MB5745.namprd12.prod.outlook.com (2603:10b6:8:5c::7) MIME-Version: 1.0 X-MS-PublicTrafficType: Email X-MS-TrafficTypeDiagnostic: DM4PR12MB5745:EE_|DS0PR12MB7583:EE_ X-MS-Office365-Filtering-Correlation-Id: d659d4fc-8125-4c02-53e5-08dac1c22618 X-MS-Exchange-SenderADCheck: 1 X-MS-Exchange-AntiSpam-Relay: 0 X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: 1HQJxggaKLt+pv7cEHFv7frLUcb9xvDp4DBGhbZPv/c/xY6A9gnQA46k1bBPhzICOhqPSRTsIp/2oOq4iAWc1G9YrJmIi1m8cOZ7hMOSDd/XUPlTJxhHSCYM/7kHq4QcwVsdGBFpdZcOLM2DPQQQzR09RFI6TUA8lujYHyk+6i+zmONB76lYH64rxavrJyOXkpE3dSFW4Xb4oWHZHp/jptlfbQFelGe9wADgHcA83Rfzef1YWkKaeUZ3TeFd5eFxQjHSRyFJ3olK/OZJcN1aKEW2t6NJ/07I5PSyFeST1IQcKdYw7Mx70gjDBJfr8VYa5xayur7RUFxF+q5HpTEnebdsO/qzKpXjopubV/fXTjNG7uaVmCavn+exafNTsJMtWYMWIUCqa534jXMCOpR5KxVRjJWh4yHTmyy8VhLIcPnlzxlUeeIAynzp/TLRuRfYycr6vPkYk8hpZTMO+cTXGTOKIxmrrajKcj7qyL8iW97IKUKbBR+S8AhrCuMaePw9I+wZG3saUZM1a4V7vZrWwJGewhd3RfJaLqzLDOYoUbYHtpRX/4kw4dfVUPRc6YcwRjSourqt+CDQRy/hLqPujfuaMMT1q6mOV95R/l4Qs3ChvF81ip4ZFed1ivk/+sLI1KZcw5Gdh8ukUWMYtAKT57Ifvgb5ohmWmuxl9UeBAXTN6cJA+hZ0xxYc1hHX1vb0ydjuWpFCINu8D7samLgjyShoc0W0TXCG8aUzRkfRP0v32w04E2XD8xbEUzjItlGZ2tFLsYt1bhk+LMt1ju+gcy1EF+XwgyQjG9oyFO6BJkUg75LC8HBFYJ+kKS0GY0qD X-Forefront-Antispam-Report: CIP:255.255.255.255;CTRY:;LANG:en;SCL:1;SRV:;IPV:NLI;SFV:NSPM;H:DM4PR12MB5745.namprd12.prod.outlook.com;PTR:;CAT:NONE;SFS:(13230022)(4636009)(376002)(346002)(39860400002)(366004)(136003)(396003)(451199015)(6512007)(2616005)(5660300002)(38100700002)(66476007)(66556008)(6506007)(66946007)(41300700001)(8676002)(6666004)(31696002)(4326008)(36756003)(83380400001)(86362001)(8936002)(110136005)(186003)(2906002)(6486002)(53546011)(31686004)(84970400001)(478600001)(316002)(43740500002)(45980500001);DIR:OUT;SFP:1101; X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1 X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?SVFyUHNpVmRZZkxDTE1QYUR4L2cveFcvbXR4QUJ2VlFLam1mc2wyMHBFU3Ba?= =?utf-8?B?VVdGUVV5NVZPQTNKaklTSVo2eWNma1J5MGZVeTF2T1ROQ2gzbExCWGhtbEtu?= =?utf-8?B?Q0llUEdQeFJML0JYMlZxc0NPQkFSY25NbUgzNXZTVE9TVElPMDViR3kvTnhV?= =?utf-8?B?NGdJbW02SW9VY0d2SU1oUEpmQ296QnFuakZ4R0ZoU1JScGt4R01pV2dJdU81?= =?utf-8?B?MCsrSHJoN2Y1akhyNWZMRDZ5ejk4cHNTOHp3RlpMc0dRdDFDZHVadVZWemVa?= =?utf-8?B?RUxFYlM2aXVoa0o4RW90WHg0RGx4dmltTGlSM3FuOGN0TzJpckZIMG9GeHlv?= =?utf-8?B?L2lWNGc1QjFmNGllSnptSUNtc0YwbDF2TTd5THV5b05vKzdDbTlnNk9SK0JG?= =?utf-8?B?YU9oRTdNb01PNnVRU1ZRa2Q0Z1k2bWMwVFZPazRhZS9JRFdTM25neW5qTkRL?= =?utf-8?B?bzduKzMxSDJHek40TEF0eEd5ZDBHaTBVdUJMZUxiTUU5dE1pM2dXMjNpZVk1?= =?utf-8?B?SlpUMWZCR2V3RUtuMjROZ1F0WVRFNlRNcDFQa2RTVEtrNElaYVhOTEFCRm1W?= =?utf-8?B?TngwUDQwby9aQ3ZRbXlnQnQ3RFNQdkt2YklySVZvekszckFhVDNhMElLNjFT?= =?utf-8?B?bzZCc1lyWmxPTE9zOW5nQ2YvS1hxSC9CaXlaN0MvY2tHaE5vQ1pmUlBrVS9T?= =?utf-8?B?QzBVTVVQODROaWNOZFRxMmx5MElWWlgvWHp4dlJiemFISVN6eVpzZWVQWXNC?= =?utf-8?B?QkRMQVpYSFBBcWw1dWNUZWNSVC9BcDlBeldJTHczekxqMXRLRzVVdVVtZUhk?= =?utf-8?B?OUtMdytCSFo1b1oyWnM5RWU4MklnbjZnYVVhcllWYW1vbXdlMllWeVZwTGRo?= =?utf-8?B?T1VhQ3ZYNVkwRFNkWU54VlN1YUMwSmc0ckJ6VEJpckRPaUdCWmwzb3dmMm1v?= =?utf-8?B?a21zY0NrbW94ZVlGMTBVc2tsdzZHY0VoSkd5UEdaUHp5QkVGU1EyWkQrbTAy?= =?utf-8?B?SUpYazUreDJjbERvQ3JnZEFoYmQvNTdJNFBaclBjUWQvUjNEamprK1hMVXJZ?= =?utf-8?B?ZFErbnlLV1BDTU45bnV2TEtxSHJObFliR0lHVlBrWElud3ZZaVpKWkxqSFQ5?= =?utf-8?B?aWxCYTZxQ1R2SjY2TG9Ib1NPbFJybzVxc0NIbnk1ZGdHNW5QTmgxL0ZaVzdC?= =?utf-8?B?aUpUWEpIZ0w2d21CQkVEdGRBL1d4R3hMZlBzV05HRzMzRGc4dFQ4TVZkcWV6?= =?utf-8?B?RHFaN3BzUHNSQUNqQTNXSWZSVjFQWE94dHNHK3N0NkE5ejBxVHhrbWJVQVN5?= =?utf-8?B?Q1U0VE9YOGlzRDFYL1hQaXoxaVg5Mk40SWIwZzZVUlB2MWVWT3JsUlJsTmF2?= =?utf-8?B?bkU5Uyt3QjduWUxlZ1VLNHRNR3ovMlFHMjFBaTNOQStkaDdHSnhsNzlkOHpR?= =?utf-8?B?R1lNNi9XZktWSnExNDBQTmtUOFF3QldiOThQQWJIeDRBNXg3OFZWMmUrcEJi?= =?utf-8?B?RlcvMWNiWGszRXNpTmxxdW13UGV2RmhWWWUvS3gyNjZocnVac1ptakh0OVRl?= =?utf-8?B?eHRFWUtCcFY0UnEyRTVLeWJzSjhiZUpZdFRKQ0x6K3JCczVoaGhHclZrcGRw?= =?utf-8?B?WDhTRi9LZW9kS0h0WTV5SkIxTUdLRUE1RlNFQUNONldqWi91dFRFNENWMXY0?= =?utf-8?B?S24vZ0ZWbU1OdlRLeXA2a05JTUMvcnJPdkc2Q2d0V2Y2K21EdEhRWnJvamdS?= =?utf-8?B?bmtmczZEV21kdlN2amRTbFdNNEM4YlZNRWRYRXdiUTdtdmJNbnlkOGxtNWFh?= =?utf-8?B?bUc5d0dKbEtoV05Vb3g5NHZhL2hXU3dPR1lxd1BTbXZWUUdCUkcydjg0Z1Bn?= =?utf-8?B?T0lnbHdOM1ZGakxYY29BVnlyY2VaYklTeWUzWVRxMmtGWkQ3T0RDVmVTbEE0?= =?utf-8?B?UENjYjRCOTZReTlUcGxtK0MxQVFsVEl6TWtDbTV5ak8rZkdiMERkSVYwcENw?= =?utf-8?B?NklhSTB6SVQ2US9XWkI4VGgzMmpoODNUUVVxcThRS0RCVENIRGRVUzJjU2lx?= =?utf-8?B?SzMzUUxDZXNTSW5QN0wyL3JLME0vRUJ4clRrenJqQ3MwM2s3YnBqSUV1Z1hB?= =?utf-8?B?VW5lRnNWNWNVRlpoWHNPdGFNMTJPME5GYVBOSlhvaEJlSzg0OHoyQUpRUzdK?= =?utf-8?Q?zkLKVDBBLKJVuoK97NifdWk=3D?= X-OriginatorOrg: amd.com X-MS-Exchange-CrossTenant-Network-Message-Id: d659d4fc-8125-4c02-53e5-08dac1c22618 X-MS-Exchange-CrossTenant-AuthSource: DM4PR12MB5745.namprd12.prod.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-OriginalArrivalTime: 08 Nov 2022 19:48:03.5634 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-CrossTenant-Id: 3dd8961f-e488-4e60-8e11-a82d994e183d X-MS-Exchange-CrossTenant-MailboxType: HOSTED X-MS-Exchange-CrossTenant-UserPrincipalName: Wpd/JD40ghRDUF4DwSW7To57OzfTsPnyKiuTcrka+MkQYGJthKm6Z0T8YZbIxP6Oa/6O2KxYRAhASs5ExvBwow== X-MS-Exchange-Transport-CrossTenantHeadersStamped: DS0PR12MB7583 X-Spam-Status: No, score=-4.9 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,NICE_REPLY_A,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H2,SPF_HELO_PASS,SPF_PASS,TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: Hi, Thanks for the review! On 08/11/2022 18:12, Tom Tromey wrote: > Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding. > > >>>>>> "Lancelot" == Lancelot SIX via Gdb-patches writes: > > Lancelot> The python code maintains a list of threads for each inferior. This > Lancelot> list is implemented as a linked list. When the number of threads grows > Lancelot> high, this implementation can begin to be a performance bottleneck as > Lancelot> finding a particular thread_object in the list has a complexity of O(N). > > Thanks for the patch. > > Lancelot> - delete tmp; > Lancelot> + auto it = inf_obj->threads->find (tp); > Lancelot> + if (it != inf_obj->threads->end ()) > Lancelot> + { > Lancelot> + it->second->thread = nullptr; > Lancelot> + inf_obj->threads->erase (it); > > Doesn't the erase implicitly also drop the 'thread' reference? > That is, I don't think the nullptr assignment is needed here. Yes, the erase does decrease the refcount associated with the object. But here "it->second->thread = nullptr" achieves another goal. *it->second is the thread_object (seen as a gdb.InferiorThread from python). it->second->thread is the link from the python object to the corresponding struct thread_info in gdb core. It is possible for the python code to hold a reference to the thread_object when this is called. As the thread_info object will soon be freed, we need to make sure that the link is cleared (otherwise we would risk use after free). This link can be later checked by thpy_is_valid (or gdb.InferiorThread.is_valid in python) I did initially miss this, and this caused a regression in gdb.python/py-inferior.exp. > > Lancelot> - PyTuple_SET_ITEM (tuple, i, thr); > Lancelot> + PyTuple_SET_ITEM (tuple, i++, thr); > > It's better to do the increment separately, in case PyTuple_SET_ITEM is > a macro that evaluates arguments more than once. Indeed, this is a good point. I'll change this before for the next revision. Best, Lancelot. > > Tom