From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 1393 invoked by alias); 20 Oct 2017 22:46:35 -0000 Mailing-List: contact fortran-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Post: List-Help: , Sender: fortran-owner@gcc.gnu.org Received: (qmail 981 invoked by uid 89); 20 Oct 2017 22:46:34 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.4 required=5.0 tests=BAYES_00,FREEMAIL_FROM,RCVD_IN_DNSWL_NONE,RCVD_IN_SORBS_SPAM,SPF_PASS autolearn=no version=3.3.2 spammy=gradually, H*MI:FCFD, H*MI:A6FA X-Spam-User: qpsmtpd, 2 recipients X-HELO: mail-wm0-f68.google.com Received: from mail-wm0-f68.google.com (HELO mail-wm0-f68.google.com) (74.125.82.68) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 20 Oct 2017 22:46:33 +0000 Received: by mail-wm0-f68.google.com with SMTP id l68so436530wmd.5; Fri, 20 Oct 2017 15:46:32 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:in-reply-to:references:mime-version :content-transfer-encoding:subject:to:cc:from:message-id; bh=z+4IXX2G3Ix5JGTuXauMeG8XiXg4anKsFYJU+KLpMwM=; b=apKNFPzNwaQrkjEV7rEdLwCRsG53mNX6f0C1G6KhyWoVSxAFuElAlsrbI+yR+QnWdd rxNGPW+EX+C1zxUxEQ2TsjFI1yHzD9slXBeQbexKSkxUdUClXVWorDOTvG/+qogebeVc hPKfoWuuBG6/glf9J1Qxyn5BKKRHV4iVBvjYAGqxcfia3P0x1rhbSFBDrEkoZ9XjSx4j yDZdVNe3IF9kYVhI4Yc+sgV78/HrD/qzdUHl+e+XwYj6qo1xOk3KahfKrCF6UzLKJMI6 PE7wq//pNdbhwbQ7X6DR6VYmAw4KIS5TQ5nEu6n1r9DFGOfN5M5WvFgHYs6V+Klc1XcE R9Zg== X-Gm-Message-State: AMCzsaW2APZ6bLN82vhM3s3cO+OdoKgkpcA6GbUnGcIWxnjXn2glZ4sp i1utHP3UIsTXAMYXllpg3pg= X-Google-Smtp-Source: ABhQp+QL/BANJ9i89Ds4LZkslh7LGNgJsk0tzgp3Cowq5JtHFnE4fLLiWGRJRAxgcQiqCQ6Oq/o6zg== X-Received: by 10.28.131.200 with SMTP id f191mr221323wmd.39.1508539590877; Fri, 20 Oct 2017 15:46:30 -0700 (PDT) Received: from [100.99.76.48] (213-225-1-61.nat.highway.a1.net. [213.225.1.61]) by smtp.gmail.com with ESMTPSA id s70sm2891816wrc.83.2017.10.20.15.46.29 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 20 Oct 2017 15:46:30 -0700 (PDT) Date: Fri, 20 Oct 2017 22:46:00 -0000 In-Reply-To: <20171019080303.3uaqy7x32dus5urx@nbbrfq.cc.univie.ac.at> References: <1448974501-30981-1-git-send-email-rep.dot.nop@gmail.com> <1448974501-30981-2-git-send-email-rep.dot.nop@gmail.com> <84A89967-93B1-47B5-8312-3BC999FEF0EA@gmail.com> <20171019080303.3uaqy7x32dus5urx@nbbrfq.cc.univie.ac.at> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Subject: Re: [PATCH] Derive interface buffers from max name length To: Janne Blomqvist CC: Fortran List ,GCC Patches From: Bernhard Reutner-Fischer Message-ID: X-IsSubscribed: yes X-SW-Source: 2017-10/txt/msg00093.txt.bz2 On 19 October 2017 10:03:06 CEST, Bernhard Reutner-Fischer wrote: >On Sat, Jun 18, 2016 at 09:46:17PM +0200, Bernhard Reutner-Fischer >wrote: >> On December 3, 2015 10:46:09 AM GMT+01:00, Janne Blomqvist > wrote: >> >On Tue, Dec 1, 2015 at 6:51 PM, Bernhard Reutner-Fischer >> > wrote: >> >> On 1 December 2015 at 15:52, Janne Blomqvist >> > wrote: >> >>> On Tue, Dec 1, 2015 at 2:54 PM, Bernhard Reutner-Fischer >> >>> wrote: >> >>>> These three function used a hardcoded buffer of 100 but would be >> >better >> >>>> off to base off GFC_MAX_SYMBOL_LEN which denotes the maximum >length >> >of a >> >>>> name in any of our supported standards (63 as of f2003 ff.). >> >>> >> >>> Please use xasprintf() instead (and free the result, or course). >One >> >>> of my backburner projects is to get rid of these static symbol >> >>> buffers, and use dynamic buffers (or the symbol table) instead. >We >> >>> IIRC already have some ugly hacks by using hashing to get around >> >>> GFC_MAX_SYMBOL_LEN when handling mangled symbols. Your patch >doesn't >> >>> make the situation worse per se, but if you're going to fix it, >lets >> >>> do it properly. >> >> >> >> I see. >> >> >> >> /scratch/src/gcc-6.0.mine/gcc/fortran$ git grep >> >> "^[[:space:]]*char[[:space:]][[:space:]]*[^[;[:space:]]*\[" | wc >-l >> >> 142 >> >> /scratch/src/gcc-6.0.mine/gcc/fortran$ git grep "xasprintf" | wc >-l >> >> 32 >> > >> >Yes, that's why it's on the TODO-list rather than on the DONE-list. >:) >> > >> >> What about memory fragmentation when switching to heap-based >> >allocation? >> >> Or is there consensus that these are in the noise compared to >other >> >> parts of the compiler? >> > >> >Heap fragmentation is an issue, yes. I'm not sure it's that >> >performance-critical, but I don't think there is any consensus. I >just >> >want to avoid ugly hacks like symbol hashing to fit within some >fixed >> >buffer. Perhaps an good compromise would be something like >std::string >> >with small string optimization, but as you have seen there is some >> >resistance to C++. But this is more relevant for mangled symbols, so >> >GFC_MAX_MANGLED_SYMBOL_LEN is more relevant here, and there's only a >> >few of them left. So, well, if you're sure that mangled symbols are >> >never copied into the buffers your patch modifies, please consider >> >your original patch Ok as well. Whichever you prefer. >> > >> >Performance-wise I think a bigger benefit would be to use the symbol >> >table more and then e.g. be able to do pointer comparisons rather >than >> >strcmp(). But that is certainly much more work. >>=20 >> Hm, worth a look indeed since that would certainly be a step in the >right direction. > >Installed the initial patch as intermediate step as r253881 for now. JFYI I'm contemplating to move the stack-based allocations to heap-based on= es now, starting with gfc_match_name and gradually moving to pointer compar= isons with the stringpool based identifiers. I'll strive to suggest somethi= ng for discussion in smallish steps when it's ready. Cheers, > >thanks, >>=20 >> > >> >> BTW: >> >> $ git grep APO >> >> io.c: static const char *delim[] =3D { "APOSTROPHE", "QUOTE", >"NONE", >> >NULL }; >> >> io.c: static const char *delim[] =3D { "APOSTROPHE", "QUOTE", >"NONE", >> >NULL }; >> > >> >? What are you saying? >>=20 >> delim is duplicated, we should remove one instance. >> thanks, >>=20