From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 100469 invoked by alias); 5 Aug 2017 18:14:33 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 100453 invoked by uid 89); 5 Aug 2017 18:14:30 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.3 required=5.0 tests=AWL,BAYES_00,RP_MATCHES_RCVD,SPF_HELO_PASS,SPF_PASS autolearn=ham version=3.3.2 spammy=lean, thousands, yours, our X-HELO: smtp.polymtl.ca Received: from smtp.polymtl.ca (HELO smtp.polymtl.ca) (132.207.4.11) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Sat, 05 Aug 2017 18:14:28 +0000 Received: from simark.ca (simark.ca [158.69.221.121]) (authenticated bits=0) by smtp.polymtl.ca (8.14.7/8.14.7) with ESMTP id v75IEMeh004773 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT) for ; Sat, 5 Aug 2017 14:14:27 -0400 Received: by simark.ca (Postfix, from userid 112) id 247AC1EA05; Sat, 5 Aug 2017 14:14:22 -0400 (EDT) Received: from simark.ca (localhost [127.0.0.1]) by simark.ca (Postfix) with ESMTP id E9D891E5B1; Sat, 5 Aug 2017 14:14:20 -0400 (EDT) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII; format=flowed Content-Transfer-Encoding: 7bit Date: Sat, 05 Aug 2017 18:14:00 -0000 From: Simon Marchi To: Sergio Durigan Junior Cc: GDB Patches , Pedro Alves , Eli Zaretskii Subject: Re: [PATCH v2] Implement the ability to set/unset environment variables to GDBserver when starting the inferior In-Reply-To: <87mv7f7x6t.fsf@redhat.com> References: <20170629194106.23070-1-sergiodj@redhat.com> <20170727033531.23066-1-sergiodj@redhat.com> <87shhc9ffa.fsf@redhat.com> <291aec4022448984a38891ddbccf08e1@polymtl.ca> <87mv7f7x6t.fsf@redhat.com> Message-ID: <7be6bdecb0da90c8b2efb550fc017a5e@polymtl.ca> X-Sender: simon.marchi@polymtl.ca User-Agent: Roundcube Webmail/1.3.0 X-Poly-FromMTA: (simark.ca [158.69.221.121]) at Sat, 5 Aug 2017 18:14:22 +0000 X-IsSubscribed: yes X-SW-Source: 2017-08/txt/msg00100.txt.bz2 On 2017-08-05 01:03, Sergio Durigan Junior wrote: > So, I've been thinking about this implementation over the last few > days, > but it's still a bit confuse to me. My C++-foo is not so good as > yours, > so maybe you can give me a hand. > > From what I understood initially, your intention was to make a new > class > that inherited from std::vector but overloaded/implemented methods to > mimic what a std::set would do. However, after reading a bit, it > doesn't seem like a good idea to inherit from std::vector (or any std > containers). Which made me realize that maybe you are talking about > creating a class that encapsulates a std::vector, without inheriting > from it, and that provided the regular .push_back, .insert, .empty, > etc., methods, but in an enhanced way in order to e.g. prevent the > insert of duplicated elements, which is one of the things we miss from > std::set. Do you mean "... we miss from std::vector"? I also read about inheriting STL containers. The danger is that they don't have virtual destructors. So let's say we have gdb::set_vector inheriting std::vector and try to delete a set_vector instance through a vector pointer, the set_vector destructor won't be called. If set_vector is simply adding some methods to the interface (no data members, no user defined destructor), I don't know if this is really a problem. But now that I think of it, if we want to make sure this object guarantees the properties of a set (like no duplicate elements), we would have to hide most of the vector interface and only expose a set-like interface. Otherwise, it would be possible to call push_back and insert a duplicate element. Given that, I'd lean towards creating a class that includes a vector and exposes a set-like interface, rather than inheriting. > > Am I in the right direction here? Also, I started to think... I don't > envision the user setting thousands of user-set environment variables, > so *maybe* using std::set would be OK-ish for our use case scenario. I > don't know. While I understand the concern about premature > pessimization, I'm also not a fan of complicating the implementation > just for a little bit of optimization either. > > WDYT? Actually, if we expected the user to set thousands of environment variables and needed it to be fast to set and unset variables, it would be good to use std::set because of the O(log(N)) lookups/insertions/removals, which matters more when you have a lot of elements. But when you just have a few elements, the constant cost is more significant. A vector-based set would have O(N) complexity for these operations (at least for insertions and removals, for lookups it depends if it is sorted), which would be bad if we had thousands of elements. But since we expect to have just a few, it would likely be faster than std::set's constant cost. But I agree with you that the important thing now is to get the algorithm/functionality right. The typical use case is to have at the maximum a few inferiors, which may have at the maximum a few environment variables defined. The defined environment variables are only used when doing a "run", which doesn't happen often. So in the end, I don't think it would make a perceptible difference in memory usage or execution time to use an std::set. If it helps to make the code significantly simpler, then I think it's worth it. And we can always revisit it later by making a drop-in replacement for std::set. Simon