From mboxrd@z Thu Jan 1 00:00:00 1970 X-Msuck: nntp://news.gmane.io/gmane.emacs.gnus.general/85242 Path: news.gmane.org!not-for-mail From: Eric Abrahamsen Newsgroups: gmane.emacs.gnus.general Subject: Re: [PATCH] Two issues with the gnus-registry Date: Sat, 08 Nov 2014 07:56:58 +0800 Message-ID: <87a942lg39.fsf@ericabrahamsen.net> References: <87egtx70hy.fsf@ericabrahamsen.net> <87wq7lsggo.fsf@lifelogs.com> <87zjchxr1v.fsf@ericabrahamsen.net> <87h9yogjer.fsf@ericabrahamsen.net> NNTP-Posting-Host: plane.gmane.org Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="=-=-=" X-Trace: ger.gmane.org 1415404381 6194 80.91.229.3 (7 Nov 2014 23:53:01 GMT) X-Complaints-To: usenet@ger.gmane.org NNTP-Posting-Date: Fri, 7 Nov 2014 23:53:01 +0000 (UTC) To: ding@gnus.org Original-X-From: ding-owner+M33486@lists.math.uh.edu Sat Nov 08 00:52:54 2014 Return-path: Envelope-to: ding-account@gmane.org Original-Received: from util0.math.uh.edu ([129.7.128.18]) by plane.gmane.org with esmtp (Exim 4.69) (envelope-from ) id 1XmtKn-0001rP-8r for ding-account@gmane.org; Sat, 08 Nov 2014 00:52:53 +0100 Original-Received: from localhost ([127.0.0.1] helo=lists.math.uh.edu) by util0.math.uh.edu with smtp (Exim 4.63) (envelope-from ) id 1XmtK4-0003Qe-Aq; Fri, 07 Nov 2014 17:52:08 -0600 Original-Received: from mx2.math.uh.edu ([129.7.128.33]) by util0.math.uh.edu with esmtps (TLSv1:AES256-SHA:256) (Exim 4.63) (envelope-from ) id 1XmtK2-0003QS-CC for ding@lists.math.uh.edu; Fri, 07 Nov 2014 17:52:06 -0600 Original-Received: from quimby.gnus.org ([80.91.231.51]) by mx2.math.uh.edu with esmtps (TLSv1:AES128-SHA:128) (Exim 4.76) (envelope-from ) id 1XmtJq-00076w-Qc for ding@lists.math.uh.edu; Fri, 07 Nov 2014 17:52:05 -0600 Original-Received: from plane.gmane.org ([80.91.229.3]) by quimby.gnus.org with esmtp (Exim 4.80) (envelope-from ) id 1XmtJo-0002bq-Qj for ding@gnus.org; Sat, 08 Nov 2014 00:51:52 +0100 Original-Received: from list by plane.gmane.org with local (Exim 4.69) (envelope-from ) id 1XmtJo-000106-7O for ding@gnus.org; Sat, 08 Nov 2014 00:51:52 +0100 Original-Received: from 103.244.102.210 ([103.244.102.210]) by main.gmane.org with esmtp (Gmexim 0.1 (Debian)) id 1AlnuQ-0007hv-00 for ; Sat, 08 Nov 2014 00:51:52 +0100 Original-Received: from eric by 103.244.102.210 with local (Gmexim 0.1 (Debian)) id 1AlnuQ-0007hv-00 for ; Sat, 08 Nov 2014 00:51:52 +0100 X-Injected-Via-Gmane: http://gmane.org/ Original-Lines: 323 Original-X-Complaints-To: usenet@ger.gmane.org X-Gmane-NNTP-Posting-Host: 103.244.102.210 User-Agent: Gnus/5.130012 (Ma Gnus v0.12) Emacs/24.4 (gnu/linux) Cancel-Lock: sha1:M9RRLow/wIhtg7ighbTiXe+OnEM= X-Spam-Score: -2.3 (--) List-ID: Precedence: bulk Xref: news.gmane.org gmane.emacs.gnus.general:85242 Archived-At: --=-=-= Content-Type: text/plain Eric Abrahamsen writes: > Eric Abrahamsen writes: > >> Ted Zlatanov writes: > > [...] > >>> The registry has ERT tests, which I thought covered this case. Can you >>> look at `tests/gnustest-registry.el'? As a first step, can you try >>> making tests to demonstrate the problems? >> >> Will do. > > Okay, turns out there were tests, but the bit that tests pruning was > commented out :) > > This is the first in a (gradual) series of patches. It does very little > except: > > 1. Adjust `registry-prune' to do what the docstring says: return the > total number of entries pruned > > 2. Uncomment the pruning test and adjust it so it correctly catches the > number of pruned entries. > > Test should fail. Over the next couple of days I'll add another test to > check that entries are sorted correctly before pruning, and then take a > stab at fixing the pruning itself. Okay I realized it didn't make too much sense to patch gnus just to make it fail, so here's a complete patch, with passing tests, that preserves precious entries when pruning. I haven't done the sorting yet, that will be a separate patch. My main realization when working on this is that the distinction between max-soft and max-hard is confusing, and possibly unnecessary. Do we really need both? If, as I suspect, most users set one but not the other, then there's no point in having both. Is anyone really setting two different values for `gnus-registry-max-entries' and `gnus-registry-max-pruned-entries'? And why? Eric --=-=-= Content-Type: text/x-diff Content-Disposition: attachment; filename=0001-Preserve-precious-entries-when-pruning-registry.patch >From d19b46f0e3afdf6f2c8a30ab083b93d7d072718e Mon Sep 17 00:00:00 2001 From: Eric Abrahamsen Date: Fri, 7 Nov 2014 19:36:57 +0800 Subject: [PATCH] Preserve precious entries when pruning registry * lisp/registry.el (registry-prune): Rewrite function to only make a single pruning pass over the registry. (registry-collect-prune-candidates): New function formerly known as `registry-prune-soft-candidates'. (registry-prune-hard-candidates): Delete function. * lisp/tests/gnustest-registry.el (gnustest-registry-pruning-test): Create independent ert test for pruning. (gnustest-registry-make-testable-db): Allow specifying the :max-soft slot. * texi/gnus.texi (Gnus Registry Setup): Documentation clarification * lisp/gnus-registry.el (gnus-registry-max-pruned-entries): Reword docstring. The pruning process was formerly not preserving precious entries. Sorting of entries to be pruned is still not implemented. --- lisp/gnus-registry.el | 3 +- lisp/registry.el | 98 +++++++++++++++++------------------------ lisp/tests/gnustest-registry.el | 41 +++++++++++------ texi/gnus.texi | 13 ++++-- 4 files changed, 80 insertions(+), 75 deletions(-) diff --git a/lisp/gnus-registry.el b/lisp/gnus-registry.el index f3b81f7..847ceb3 100644 --- a/lisp/gnus-registry.el +++ b/lisp/gnus-registry.el @@ -243,7 +243,8 @@ the Bit Bucket." (integer :format "Maximum number: %v"))) (defcustom gnus-registry-max-pruned-entries nil - "Maximum number of pruned entries in the registry, nil for unlimited." + "Number of entries that will be retained in the registry after +pruning, nil for unlimited." :version "24.1" :group 'gnus-registry :type '(radio (const :format "Unlimited " nil) diff --git a/lisp/registry.el b/lisp/registry.el index dbc7b51..d9fb0f5 100644 --- a/lisp/registry.el +++ b/lisp/registry.el @@ -57,14 +57,13 @@ ;; Note that whether a field has one or many pieces of data, the data ;; is always a list of values. -;; The user decides which fields are "precious", F2 for example. At -;; PRUNE TIME (when the :prune-function is called), the registry will -;; trim any entries without the F2 field until the size is :max-soft -;; or less. No entries with the F2 field will be removed at PRUNE -;; TIME. - -;; When an entry is inserted, the registry will reject new entries -;; if they bring it over the max-hard limit, even if they have the F2 +;; The user decides which fields are "precious", F2 for example. At +;; PRUNE TIME, the registry will attempt to trim entries until the +;; size is (- :max-soft (* registry-size :prune-factor). No entries +;; with the F2 field will be removed at PRUNE TIME. + +;; When an entry is inserted, the registry will reject new entries if +;; they bring it over the :max-hard limit, even if they have the F2 ;; field. ;; The user decides which fields are "tracked", F1 for example. Any @@ -115,7 +114,8 @@ :initform 0.1 :type float :custom float - :documentation "At the max-hard limit, prune size * this entries.") + :documentation "Prune to \(size * :prune-factor\) less than + the :max-soft limit.") (tracked :initarg :tracked :initform nil :type t @@ -312,58 +312,42 @@ Errors out if the key exists already." (registry-lookup-secondary-value db tr val value-keys)))) (oref db :data)))))) -(defmethod registry-prune ((db registry-db) &optional sortfun) - "Prunes the registry-db object THIS. -Removes only entries without the :precious keys if it can, -then removes oldest entries first. -Returns the number of deleted entries. -If SORTFUN is given, tries to keep entries that sort *higher*. -SORTFUN is passed only the two keys so it must look them up directly." - (dolist (collector '(registry-prune-soft-candidates - registry-prune-hard-candidates)) - (let* ((size (registry-size db)) - (collected (funcall collector db)) - (limit (nth 0 collected)) - (candidates (nth 1 collected)) - ;; sort the candidates if SORTFUN was given - (candidates (if sortfun (sort candidates sortfun) candidates)) - (candidates-count (length candidates)) - ;; are we over max-soft? - (prune-needed (> size limit))) - - ;; while we have more candidates than we need to remove... - (while (and (> candidates-count (- size limit)) candidates) - (decf candidates-count) - (setq candidates (cdr candidates))) - - (registry-delete db candidates nil) - (length candidates)))) - -(defmethod registry-prune-soft-candidates ((db registry-db)) - "Collects pruning candidates from the registry-db object THIS. -Proposes only entries without the :precious keys." +(defmethod registry-prune ((db registry-db)) + "Prunes the registry-db object DB. + +Attempts to prune the number of entries down to \(* +registry-size :prune-factor\) less than the max-soft limit, so +pruning doesn't need to happen on every save. Removes only +entries without the :precious keys, so it may not be possible to +reach the target limit. + +Returns the number of deleted entries." + (let* ((size (registry-size db)) + (limit (max 0 (- (min (oref db :max-hard) + (oref db :max-soft)) + (* size (oref db :prune-factor))))) + candidates) + (if (> limit 0) + (progn + (setq candidates + (registry-collect-prune-candidates db (- size limit))) + (length (registry-delete db candidates nil))) + 0))) + +(defmethod registry-collect-prune-candidates ((db registry-db) limit) + "Collects pruning candidates from the registry-db object DB. + +Proposes only entries without the :precious keys, and attempts to +return LIMIT such candidates." (let* ((precious (oref db :precious)) (precious-p (lambda (entry-key) (cdr (memq (car entry-key) precious)))) (data (oref db :data)) - (limit (oref db :max-soft)) - (candidates (loop for k being the hash-keys of data - using (hash-values v) - when (notany precious-p v) - collect k))) - (list limit candidates))) - -(defmethod registry-prune-hard-candidates ((db registry-db)) - "Collects pruning candidates from the registry-db object THIS. -Proposes any entries over the max-hard limit minus size * prune-factor." - (let* ((data (oref db :data)) - ;; prune to (size * prune-factor) below the max-hard limit so - ;; we're not pruning all the time - (limit (max 0 (- (oref db :max-hard) - (* (registry-size db) (oref db :prune-factor))))) - (candidates (loop for k being the hash-keys of data - collect k))) - (list limit candidates))) + (candidates (cl-loop for k being the hash-keys of data + using (hash-values v) + when (notany precious-p v) + collect k))) + (delq nil (cl-subseq candidates 0 limit)))) (provide 'registry) ;;; registry.el ends here diff --git a/lisp/tests/gnustest-registry.el b/lisp/tests/gnustest-registry.el index 174a0cb..d7fac59 100644 --- a/lisp/tests/gnustest-registry.el +++ b/lisp/tests/gnustest-registry.el @@ -52,20 +52,20 @@ (should-not (registry--match :member entry '((hello))))) (message "Done with matching testing.")) -(defun gnustest-registry-make-testable-db (n &optional name file) +(defun gnustest-registry-make-testable-db (n &optional max-soft name file) (let* ((db (registry-db (or name "Testing") :file (or file "unused") :max-hard n - :max-soft 0 ; keep nothing not precious + :max-soft (or max-soft 0) :precious '(extra more-extra) :tracked '(sender subject groups)))) (dotimes (i n) (registry-insert db i `((sender "me") (subject "about you") - (more-extra) ; empty data key should be pruned - ;; first 5 entries will NOT have this extra data - ,@(when (< 5 i) (list (list 'extra "more data"))) + (more-extra) ; Empty data key should be pruned. + ;; First 5 entries will NOT have this extra data. + ,@(when (< 4 i) (list (list 'extra "more data"))) (groups ,(number-to-string i))))) db)) @@ -101,17 +101,30 @@ (should (= n (length (registry-search db :all t)))) (message "Secondary search after delete") (should (= n (length (registry-lookup-secondary-value db 'sender "me")))) - ;; (message "Pruning") - ;; (let* ((tokeep (registry-search db :member '((extra "more data")))) - ;; (count (- n (length tokeep))) - ;; (pruned (registry-prune db)) - ;; (prune-count (length pruned))) - ;; (message "Expecting to prune %d entries and pruned %d" - ;; count prune-count) - ;; (should (and (= count 5) - ;; (= count prune-count)))) (message "Done with usage testing."))) +(ert-deftest gnustest-registry-pruning-test () + "Check that precious entries are never pruned." + (let ((dbs (list + ;; Can prune fully without touching precious entries. + (gnustest-registry-make-testable-db 10 8) + ;; Pruning limited by precious entries. + (gnustest-registry-make-testable-db 10 4)))) + (dolist (db dbs) + (message "Pruning") + (let* ((size (registry-size db)) + (max-soft (oref db :max-soft)) + (keepers (registry-search db :member '((extra "more data")))) + (desired-prune-count (- size (- max-soft (* size 0.1)))) + (expected-prune-count (min (- size (length keepers)) + desired-prune-count)) + (actual-prune-count (registry-prune db))) + (ert-info + ((format "Expected to prune %d entries but pruned %d" + expected-prune-count actual-prune-count) + :prefix "Error: ") + (should (= expected-prune-count actual-prune-count))))))) + (ert-deftest gnustest-registry-persistence-test () (let* ((n 100) (tempfile (make-temp-file "registry-persistence-")) diff --git a/texi/gnus.texi b/texi/gnus.texi index 6f47786..6b687f7 100644 --- a/texi/gnus.texi +++ b/texi/gnus.texi @@ -25942,12 +25942,19 @@ the word ``archive'' is not followed. @defvar gnus-registry-max-entries The number (an integer or @code{nil} for unlimited) of entries the -registry will keep. +registry will keep. If the registry has reached or exceeded this +size, it will reject insertion of new entries. @end defvar @defvar gnus-registry-max-pruned-entries -The maximum number (an integer or @code{nil} for unlimited) of entries -the registry will keep after pruning. +The number (an integer or @code{nil} for unlimited) of entries the +registry will be pruned to when saving. In order to prevent constant +pruning, the registry will be pruned back to somewhat less than this +number. Entries with keys marked as precious will not be pruned. It +may make more sense to control the size of the registry with this +variable rather than @code{gnus-registry-max-entries}, as it won't +intefere with the creation or preservation of entries with precious +keys. @end defvar @defvar gnus-registry-cache-file -- 2.1.3 --=-=-=--