From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mp2 ([2001:41d0:2:4a6f::]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) by ms11 with LMTPS id GE43OjRSlF/eSgAA0tVLHw (envelope-from ) for ; Sat, 24 Oct 2020 16:11:32 +0000 Received: from aspmx1.migadu.com ([2001:41d0:2:4a6f::]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) by mp2 with LMTPS id sCwLNjRSlF/9LgAAB5/wlQ (envelope-from ) for ; Sat, 24 Oct 2020 16:11:32 +0000 Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by aspmx1.migadu.com (Postfix) with ESMTPS id 6300D9404CC for ; Sat, 24 Oct 2020 16:11:32 +0000 (UTC) Received: from localhost ([::1]:59102 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1kWM8f-0002ed-QR for larch@yhetil.org; Sat, 24 Oct 2020 12:11:29 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:52608) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1kWM8J-0002eT-3b for emacs-orgmode@gnu.org; Sat, 24 Oct 2020 12:11:07 -0400 Received: from coral.adamspiers.org ([85.119.82.20]:36730) by eggs.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1kWM8H-0007Hl-2n; Sat, 24 Oct 2020 12:11:06 -0400 Received: from localhost (243.103.2.81.in-addr.arpa [81.2.103.243]) by coral.adamspiers.org (Postfix) with ESMTPSA id 326EC2E79A; Sat, 24 Oct 2020 17:11:00 +0100 (BST) Date: Sat, 24 Oct 2020 17:10:59 +0100 From: Adam Spiers To: Bastien Subject: Re: [PATCH] [WIP] org-agenda.el: Make org-entries-lessp more efficient Message-ID: <20201024161059.zokctln74l5rxvms@ionian.linksys.moosehall> X-OS: GNU/Linux References: <20201019011159.4484-1-orgmode@adamspiers.org> <877drfonyy.fsf@gnu.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8; format=flowed Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <877drfonyy.fsf@gnu.org> Received-SPF: pass client-ip=85.119.82.20; envelope-from=orgmode@adamspiers.org; helo=coral.adamspiers.org X-detected-operating-system: by eggs.gnu.org: First seen = 2020/10/24 12:11:00 X-ACL-Warn: Detected OS = Linux 2.2.x-3.x [generic] X-Spam_score_int: -18 X-Spam_score: -1.9 X-Spam_bar: - X-Spam_report: (-1.9 / 5.0 requ) BAYES_00=-1.9, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: emacs-orgmode@gnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: "General discussions about Org-mode." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: emacs-orgmode@gnu.org Errors-To: emacs-orgmode-bounces+larch=yhetil.org@gnu.org Sender: "Emacs-orgmode" X-Scanner: scn0 Authentication-Results: aspmx1.migadu.com; dkim=none; dmarc=fail reason="SPF not aligned (relaxed), No valid DKIM" header.from=adamspiers.org (policy=none); spf=pass (aspmx1.migadu.com: domain of emacs-orgmode-bounces@gnu.org designates 209.51.188.17 as permitted sender) smtp.mailfrom=emacs-orgmode-bounces@gnu.org X-Spam-Score: -0.91 X-TUID: UyKTZXEZpF5A On Sat, Oct 24, 2020 at 01:36:05PM +0200, Bastien wrote: >Hi Adam, > >this looks good to me, thanks a lot. Thanks for the review :-) >Does "WIP" means that you want to wait for other patches to complete >this one or shall I apply this one already? The intention is for it to be a standalone patch, so I don't think there's any need to wait for other patches. The "WIP" was coming more from the fact that I haven't done extensive testing to make sure I didn't break a single one of the many sorting criteria - especially this one: + ('category-keep (if (org-cmp-category a b) +1 nil)) ;; FIXME: check this Ideally there would already be unit and/or functional tests covering agenda sorting, but I can't see any and I imagine they would be non-trivial to add (although perhaps not too difficult either). Also, the previous behaviour was to silently ignore user-defined-{up,down} if org-agenda-cmp-user-defined was not defined, but that intuitively felt wrong to me (IMHO it kind of violates the Principle of Least Surprise) so I didn't bother to preserve that behaviour. However I admit that is a pretty subjective choice, so if you think the existing behaviour should be preserved, I can tweak the code to do that. In fact, even if you agree with me that it would be better to generate an error when user-defined-{up,down} is used with org-agenda-cmp-user-defined being nil, I just noticed that it currently generates the very unhelpful error: org-entries-cmp-1: Symbol’s function definition is void: nil so we would want to explicitly catch that case and generate a more helpful error message, e.g. "org-agenda-sorting-strategy contains user-defined sorting but org-agenda-cmp-user-defined is nil". BTW, as you can partially see from the below link, I did some performance profiling relating to this change and it did indeed seem to shave a few milliseconds off org-entries-lessp, although in the grand scheme of things, that didn't make as much of a dent in org-agenda's running time as I'd hoped for ;-) https://gist.github.com/trishume/40bf7045a23412d4c016f5e8533437db#gistcomment-3494087