From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!not-for-mail From: Stefan Monnier Newsgroups: gmane.emacs.devel Subject: Re: Refactoring of emacs-lisp/autoload.el Date: Tue, 12 Aug 2008 16:09:04 -0400 Message-ID: References: <20080812162333.GA7999@muc.de> NNTP-Posting-Host: lo.gmane.org Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii X-Trace: ger.gmane.org 1218571865 28059 80.91.229.12 (12 Aug 2008 20:11:05 GMT) X-Complaints-To: usenet@ger.gmane.org NNTP-Posting-Date: Tue, 12 Aug 2008 20:11:05 +0000 (UTC) Cc: Glenn Morris , Chong Yidong , emacs-devel@gnu.org To: Alan Mackenzie Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Tue Aug 12 22:11:54 2008 Return-path: Envelope-to: ged-emacs-devel@m.gmane.org Original-Received: from lists.gnu.org ([199.232.76.165]) by lo.gmane.org with esmtp (Exim 4.50) id 1KT0Dg-00007n-Rt for ged-emacs-devel@m.gmane.org; Tue, 12 Aug 2008 22:11:53 +0200 Original-Received: from localhost ([127.0.0.1]:51516 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1KT0Cj-0000K8-Sq for ged-emacs-devel@m.gmane.org; Tue, 12 Aug 2008 16:10:53 -0400 Original-Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1KT0B5-0008AD-4m for emacs-devel@gnu.org; Tue, 12 Aug 2008 16:09:11 -0400 Original-Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1KT0B3-00089B-6S for emacs-devel@gnu.org; Tue, 12 Aug 2008 16:09:10 -0400 Original-Received: from [199.232.76.173] (port=54700 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1KT0B3-000894-0x for emacs-devel@gnu.org; Tue, 12 Aug 2008 16:09:09 -0400 Original-Received: from smtp-04.arnet.com.ar ([200.45.191.26]:49771) by monty-python.gnu.org with smtp (Exim 4.60) (envelope-from ) id 1KT0B2-0005Xu-2R for emacs-devel@gnu.org; Tue, 12 Aug 2008 16:09:08 -0400 Original-Received: (qmail 7445 invoked from network); 12 Aug 2008 20:07:19 -0000 Original-Received: from unknown (HELO ceviche.home) (190.30.131.157) by 0 with SMTP; 12 Aug 2008 20:07:18 -0000 Original-Received: by ceviche.home (Postfix, from userid 20848) id 94502B40A9; Tue, 12 Aug 2008 16:09:04 -0400 (EDT) In-Reply-To: <20080812162333.GA7999@muc.de> (Alan Mackenzie's message of "Tue, 12 Aug 2008 16:23:33 +0000") User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/23.0.60 (gnu/linux) X-detected-kernel: by monty-python.gnu.org: Linux 2.6 (newer, 3) X-BeenThere: emacs-devel@gnu.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: "Emacs development discussions." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Original-Sender: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Errors-To: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Xref: news.gmane.org gmane.emacs.devel:102377 Archived-At: > ######################################################################### > (i) It is better structured: > o - the functions are coherently single purpose to a greater extent than > before; > o - the output file handling has been separated from source file > considerations; > o - two `catch' constructs (~120 and ~80 lines) have been eliminated. Sounds very good. > (ii) It produces more consistent result in the comment sections of loaddefs.el > etc. In particular: > o - The lines that identify the source file now always (rather than just > sometimes) give a file name relative to the "starting directory" (usually > .../lisp). E.g.: > *** calc/calc-loaddefs.el.old 2008-08-12 13:17:32.983291472 +0000 > --- calc/calc-loaddefs.el.new 2008-08-12 15:00:26.468779024 +0000 > *************** > *** 9,16 **** > ;;;;;; math-read-preprocess-string calcDigit-edit calcDigit-algebraic > ;;;;;; calc-alg-digit-entry calc-do-alg-entry calc-alg-entry calc-algebraic-entry > ;;;;;; calc-auto-algebraic-entry calc-do-calc-eval calc-do-quick-calc) > ! ;;;;;; "calc-aent" "calc-aent.el" "397561d73c948bd9256db97c177e84f6") > ! ;;; Generated autoloads from calc-aent.el > (autoload 'calc-do-quick-calc "calc-aent" "\ > Not documented > --- 9,16 ---- > ;;;;;; math-read-preprocess-string calcDigit-edit calcDigit-algebraic > ;;;;;; calc-alg-digit-entry calc-do-alg-entry calc-alg-entry calc-algebraic-entry > ;;;;;; calc-auto-algebraic-entry calc-do-calc-eval calc-do-quick-calc) > ! ;;;;;; "calc-aent" "calc/calc-aent.el" "397561d73c948bd9256db97c177e84f6") > ! ;;; Generated autoloads from calc/calc-aent.el I'm not sure why you think it's an improvement: the previous behavior was to use a file name relate to where the name was placed (i.e. relative to the file in which the entry is created). In my opinions, relative file names placed in files should generally be relative to the file in which they appear rather than relative to the "top of the project". Of course, there might be a good reason for your change, so I'm not necessarily opposed to this. More specifically, I think in this particular instance it does not matter either way, so I don't have a strong opinion. I'd still be interested to hear your reason for this change. Just a personal preference? Fix an actual problem? Simplifies the code? > o - The final section (which records files which had no autoload symbols) no > longer includes any files for which there is a normal section higher up. > For example, in lisp/loaddefs.el at the moment, "calc/calc-aent.el" > violates this rule. I have assumed that this is a bug. No, this was not a bug. This section is present to speed up the refresh of the loaddefs.el file, so that files that don't have other entries in loaddefs.el don't have to be opened&scanned when they haven't changed since the last time we refreshed loaddefs.el. So calc/calc-aent.el should probably be mentioned in this list so as to avoid having to open&scan it when it hasn't changed. > (iii) The new autoload.el runs quite a lot faster than the old one. :-) Here > are some comparitive timings, done under fair conditions on my 1.2 GHz > Athlon box: > OLD: NEW: > real 1m11.502s real 0m40.729s > user 0m55.141s user 0m24.519s > sys 0m15.981s sys 0m15.998s There are 2 ways to run this code: one is to rebuild loaddefs.el from scratch, the other is to update a preexisting loaddefs.el (typically just after "cvs update"). With "make bootstrap" you care about the first, otherwise you care about the second. I.e. I care about the second ;-) The second case used to be unusably slow and I made it a lot faster around Emacs-21 (or so), so now it's fast enough: please test it after changing a handful of files, just to make sure that it's not significantly slower than before. > This is a speedup of ~75%. That's great. In parallel bootstrap builds, the loaddefs.el is sometimes on the critical path, so that's a very welcome improvement. I can't easily browse your patch with the machine I'm using right now, so I can't comment on the actual code. The current code (mostly due to yours truly, sadly) is not very satisfactory indeed, so as long as the new code is fully compatible with the various ways autoload.el is used (i..e not just by Emacs itself but by other external 3rd party packages), and as long as it doesn't significantly slow down the "no/few changes" case, I see no reason not to install it, Stefan