From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!not-for-mail From: David Kastrup Newsgroups: gmane.emacs.devel Subject: Re: Should we have a commit size guideline? Date: Tue, 15 Dec 2015 15:23:56 +0100 Message-ID: <87zixb6b9f.fsf@fencepost.gnu.org> References: NNTP-Posting-Host: plane.gmane.org Mime-Version: 1.0 Content-Type: text/plain X-Trace: ger.gmane.org 1450189459 10772 80.91.229.3 (15 Dec 2015 14:24:19 GMT) X-Complaints-To: usenet@ger.gmane.org NNTP-Posting-Date: Tue, 15 Dec 2015 14:24:19 +0000 (UTC) Cc: emacs-devel To: Artur Malabarba Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Tue Dec 15 15:24:16 2015 Return-path: Envelope-to: ged-emacs-devel@m.gmane.org Original-Received: from lists.gnu.org ([208.118.235.17]) by plane.gmane.org with esmtp (Exim 4.69) (envelope-from ) id 1a8qWM-0008Pb-Jm for ged-emacs-devel@m.gmane.org; Tue, 15 Dec 2015 15:24:06 +0100 Original-Received: from localhost ([::1]:37100 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1a8qWL-00046x-Js for ged-emacs-devel@m.gmane.org; Tue, 15 Dec 2015 09:24:05 -0500 Original-Received: from eggs.gnu.org ([2001:4830:134:3::10]:59037) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1a8qWG-00043x-P0 for emacs-devel@gnu.org; Tue, 15 Dec 2015 09:24:01 -0500 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1a8qWF-0004fp-J3 for emacs-devel@gnu.org; Tue, 15 Dec 2015 09:24:00 -0500 Original-Received: from fencepost.gnu.org ([2001:4830:134:3::e]:54788) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1a8qWD-0004fV-Qb; Tue, 15 Dec 2015 09:23:57 -0500 Original-Received: from localhost ([127.0.0.1]:40373 helo=lola) by fencepost.gnu.org with esmtp (Exim 4.82) (envelope-from ) id 1a8qWD-0004Bh-9S; Tue, 15 Dec 2015 09:23:57 -0500 Original-Received: by lola (Postfix, from userid 1000) id E32DADFFC5; Tue, 15 Dec 2015 15:23:56 +0100 (CET) In-Reply-To: (Artur Malabarba's message of "Tue, 15 Dec 2015 13:48:28 +0000") User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/25.1.50 (gnu/linux) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 2001:4830:134:3::e X-BeenThere: emacs-devel@gnu.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: "Emacs development discussions." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Original-Sender: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Xref: news.gmane.org gmane.emacs.devel:196304 Archived-At: Artur Malabarba writes: > 2015-12-15 12:49 GMT+00:00 David Kastrup : >> [Regarding commit 2e84888] >> >> This is a very, very large commit. It should have been split into >> multiple commits addressing separate issues. > > When commiting changes, I usually group them into the smallest > possible commits while still leaving everything in a consistent state > (i.e., not defining a function that's only used in later commits, not > changing a function without making the necessary changes in other > places that call this function). I find that this helps with both > git-bisect and git-revert. Sure. But that commit is not in that class. -(defconst dir-locals-file ".dir-locals.el" +(defconst dir-locals-file ".dir-locals*.el" is a change in default that is independent from the implementation and that may or may not be the main cause of the performance regression, or be responsible for some part of it. As long as it is mixed in with the rest, it's not easy to find out. - (message ".dir-locals error: %s" (error-message-string err)) + (message "%s error: %s" dir-locals-file (error-message-string err)) is a bug fix that is independent from the implementation. (when (and (string-prefix-p (car elt) file - (memq system-type - '(windows-nt cygwin ms-dos))) - (> (length (car elt)) (length (car dir-elt)))) - (setq dir-elt elt))) + (memq system-type + '(windows-nt cygwin ms-dos))) + (> (length (car elt)) (length (car dir-elt)))) + (setq dir-elt elt))) (if (and dir-elt is a gratuitous spacing change. So is most of (if (and dir-elt - (or (null locals-file) - (<= (length (file-name-directory locals-file)) - (length (car dir-elt))))) - ;; Found a potential cache entry. Check validity. - ;; A cache entry with no MTIME is assumed to always be valid - ;; (ie, set directly, not from a dir-locals file). - ;; Note, we don't bother to check that there is a matching class - ;; element in dir-locals-class-alist, since that's done by - ;; dir-locals-set-directory-class. - (if (or (null (nth 2 dir-elt)) - (let ((cached-file (expand-file-name dir-locals-file-name - (car dir-elt)))) - (and (file-readable-p cached-file) - (equal (nth 2 dir-elt) - (nth 5 (file-attributes cached-file)))))) - ;; This cache entry is OK. - dir-elt - ;; This cache entry is invalid; clear it. - (setq dir-locals-directory-cache - (delq dir-elt dir-locals-directory-cache)) - ;; Return the first existing dir-locals file. Might be the same - ;; as dir-elt's, might not (eg latter might have been deleted). - locals-file) + (or (null locals-dir) + (<= (length locals-dir) + (length (car dir-elt))))) + ;; Found a potential cache entry. Check validity. + ;; A cache entry with no MTIME is assumed to always be valid + ;; (ie, set directly, not from a dir-locals file). + ;; Note, we don't bother to check that there is a matching class + ;; element in dir-locals-class-alist, since that's done by + ;; dir-locals-set-directory-class. + (if (or (null (nth 2 dir-elt)) + (let ((cached-files (dir-locals--all-files (car dir-elt)))) + ;; The entry MTIME should match the most recent + ;; MTIME among matching files. + (and cached-files + (= (time-to-seconds (nth 2 dir-elt)) + (apply #'max (mapcar (lambda (f) (time-to-seconds (nth 5 (file-attributes f)))) + cached-files)))))) + ;; This cache entry is OK. + dir-elt + ;; This cache entry is invalid; clear it. + (setq dir-locals-directory-cache + (delq dir-elt dir-locals-directory-cache)) + ;; Return the first existing dir-locals file. Might be the same + ;; as dir-elt's, might not (eg latter might have been deleted). + locals-file) ;; No cache entry. That makes it harder to find the wheat in the chaff. Basically, one has to use git log -w -p or equivalent in order not to get distracted in unrelated content. And even then stuff is a bit arduous. -- David Kastrup