* [PATCH] Two issues with the gnus-registry
@ 2014-10-24 19:04 Eric Abrahamsen
2014-10-24 20:56 ` Eric Abrahamsen
2014-10-27 15:03 ` Ted Zlatanov
0 siblings, 2 replies; 24+ messages in thread
From: Eric Abrahamsen @ 2014-10-24 19:04 UTC (permalink / raw)
To: ding
Hi there,
I'm using the gnus-registry in Gnorb to keep track of correspondences
between Gnus messages and Org headings, using a key on the registry
entries called 'gnorb-ids. This is meant to be a "precious" key, ie
entries with this key are not pruned. So far so good:
(oref gnus-registry-db :precious) => (gnorb-ids mark)
But the entries still do get pruned! I've just had tracked entries start
disappearing on me, and realized that they're getting pruned from the
registry when I save it. There appear to be two problems here:
1. The pruning doesn't start with the oldest entries first. The
docstring of `registry-prune' says it will, but looking over the code in
both registry.el and gnus-registry.el, I don't see how that would
happen. The entries aren't sorted as they're entered, nor does
`gnus-registry-save' pass a sortfun in to `registry-save'. In fact, the
entries I'm losing are the most recently-created ones.
2. The bigger problem is that "precious" entries still seem to get
pruned. This is really difficult to edebug, because of the enormous
loops involved, and because any time the cursor passes the "db" variable
and tries to pretty-print it, Emacs runs out of memory (my registry has
about 18,000 entries in it). But something is still wrong here!
I added a sortfun to the `gnus-registry-save' call, but obviously it
slows the save process *way* the heck down. I'm not sure what else to
do, though, since (as far as I know) hash tables aren't guaranteed to
keep their sort order. Is that correct?
And I really don't know why the precious entries are getting pruned.
In a day or so, when I have more time, I'll test with a dummy registry,
with max-entries set to 10 and 'creation-time added to the precious
entries -- should make debugging easier.
Thanks,
eric
Here's the version of gnus-registry-save with the sort routine added:
(defun gnus-registry-save (&optional file db)
"Save the registry cache file."
(interactive)
(let ((file (or file gnus-registry-cache-file))
(db (or db gnus-registry-db)))
(gnus-message 5 "Saving Gnus registry (%d entries) to %s..."
(registry-size db) file)
(registry-prune db
(lambda (l r)
(let ((l-time
(car (gnus-registry-get-id-key l 'creation-time)))
(r-time
(car (gnus-registry-get-id-key r 'creation-time))))
;; Put oldest entries at the front of the
;; list.
(time-less-p r-time l-time))))
;; TODO: call (gnus-string-remove-all-properties v) on all elements?
(eieio-persistent-save db file)
(gnus-message 5 "Saving Gnus registry (size %d) to %s...done"
(registry-size db) file)))
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH] Two issues with the gnus-registry
2014-10-24 19:04 [PATCH] Two issues with the gnus-registry Eric Abrahamsen
@ 2014-10-24 20:56 ` Eric Abrahamsen
2014-10-25 19:59 ` Eric Abrahamsen
2014-10-27 15:03 ` Ted Zlatanov
1 sibling, 1 reply; 24+ messages in thread
From: Eric Abrahamsen @ 2014-10-24 20:56 UTC (permalink / raw)
To: ding
Eric Abrahamsen <eric@ericabrahamsen.net> writes:
> Hi there,
>
> I'm using the gnus-registry in Gnorb to keep track of correspondences
> between Gnus messages and Org headings, using a key on the registry
> entries called 'gnorb-ids. This is meant to be a "precious" key, ie
> entries with this key are not pruned. So far so good:
>
> (oref gnus-registry-db :precious) => (gnorb-ids mark)
>
> But the entries still do get pruned! I've just had tracked entries start
> disappearing on me, and realized that they're getting pruned from the
> registry when I save it. There appear to be two problems here:
[...]
> And I really don't know why the precious entries are getting pruned.
> In a day or so, when I have more time, I'll test with a dummy registry,
> with max-entries set to 10 and 'creation-time added to the precious
> entries -- should make debugging easier.
Okay, I got curious (frustrated) and tried this, and I just don't get
it. `registry-prune' doesn't seem to work as advertised: first it
deletes according to `registry-prune-soft-candidates', which preserves
precious entries, and then it deletes according to
`registry-prune-hard-candidates', which doesn't.p
The second pass unconditionally caps the registry length at 90% of
`gnus-registry-max-entries', without consulting the entries' precious
status at all.
The `prune-needed' local variable also appears unused.
My understanding of the process is that the two functions should create
their two separate sets (entries over 90% of
`gnus-registry-max-entries', and non-precious entries), and then the
intersection of those two sets should be deleted.
Done this way, sorting would be cheaper as well -- you'd only need to
sort candidates by age for the hard-prune set.
I'm still pretty confused, but I can prepare a patch for this. Perhaps
I'm misreading the code, but I can't see how!
Eric
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH] Two issues with the gnus-registry
2014-10-24 20:56 ` Eric Abrahamsen
@ 2014-10-25 19:59 ` Eric Abrahamsen
0 siblings, 0 replies; 24+ messages in thread
From: Eric Abrahamsen @ 2014-10-25 19:59 UTC (permalink / raw)
To: ding
Eric Abrahamsen <eric@ericabrahamsen.net> writes:
> Eric Abrahamsen <eric@ericabrahamsen.net> writes:
>
>> Hi there,
>>
>> I'm using the gnus-registry in Gnorb to keep track of correspondences
>> between Gnus messages and Org headings, using a key on the registry
>> entries called 'gnorb-ids. This is meant to be a "precious" key, ie
>> entries with this key are not pruned. So far so good:
>>
>> (oref gnus-registry-db :precious) => (gnorb-ids mark)
>>
>> But the entries still do get pruned! I've just had tracked entries start
>> disappearing on me, and realized that they're getting pruned from the
>> registry when I save it. There appear to be two problems here:
>
> [...]
>
>> And I really don't know why the precious entries are getting pruned.
>> In a day or so, when I have more time, I'll test with a dummy registry,
>> with max-entries set to 10 and 'creation-time added to the precious
>> entries -- should make debugging easier.
>
> Okay, I got curious (frustrated) and tried this, and I just don't get
> it. `registry-prune' doesn't seem to work as advertised: first it
> deletes according to `registry-prune-soft-candidates', which preserves
> precious entries, and then it deletes according to
> `registry-prune-hard-candidates', which doesn't.p
>
> The second pass unconditionally caps the registry length at 90% of
> `gnus-registry-max-entries', without consulting the entries' precious
> status at all.
>
> The `prune-needed' local variable also appears unused.
Given the commentary in registry.el, I wonder if the whole pruning
arrangement actually never quite got finished:
;; 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.
It looks like there's supposed to be a :prune-function slot on registry
objects, and presumably the Gnus registry would have a prune function
that worked more like what the comments above outline.
I'd be happy to help work on this, with a little direction...
Eric
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH] Two issues with the gnus-registry
2014-10-24 19:04 [PATCH] Two issues with the gnus-registry Eric Abrahamsen
2014-10-24 20:56 ` Eric Abrahamsen
@ 2014-10-27 15:03 ` Ted Zlatanov
2014-10-27 19:15 ` Eric Abrahamsen
1 sibling, 1 reply; 24+ messages in thread
From: Ted Zlatanov @ 2014-10-27 15:03 UTC (permalink / raw)
To: ding
On Fri, 24 Oct 2014 12:04:25 -0700 Eric Abrahamsen <eric@ericabrahamsen.net> wrote:
EA> I'm using the gnus-registry in Gnorb to keep track of correspondences
EA> between Gnus messages and Org headings, using a key on the registry
EA> entries called 'gnorb-ids. This is meant to be a "precious" key, ie
EA> entries with this key are not pruned. So far so good:
EA> (oref gnus-registry-db :precious) => (gnorb-ids mark)
EA> But the entries still do get pruned! I've just had tracked entries start
EA> disappearing on me, and realized that they're getting pruned from the
EA> registry when I save it. There appear to be two problems here:
EA> 1. The pruning doesn't start with the oldest entries first. The
EA> docstring of `registry-prune' says it will, but looking over the code in
EA> both registry.el and gnus-registry.el, I don't see how that would
EA> happen. The entries aren't sorted as they're entered, nor does
EA> `gnus-registry-save' pass a sortfun in to `registry-save'. In fact, the
EA> entries I'm losing are the most recently-created ones.
Huh. I could've sworn this worked. It must be my mistake.
EA> 2. The bigger problem is that "precious" entries still seem to get
EA> pruned. This is really difficult to edebug, because of the enormous
EA> loops involved, and because any time the cursor passes the "db" variable
EA> and tries to pretty-print it, Emacs runs out of memory (my registry has
EA> about 18,000 entries in it). But something is still wrong here!
Yeah, keeping the whole database in a big data structure is pretty bad.
Plus I wrote that code in my "loop-happy" period. I'd like to use
something better to store these records.
EA> I added a sortfun to the `gnus-registry-save' call, but obviously it
EA> slows the save process *way* the heck down. I'm not sure what else to
EA> do, though, since (as far as I know) hash tables aren't guaranteed to
EA> keep their sort order. Is that correct?
Right. But you don't need to sort the whole database, only the
non-precious entries. So it should be: (prune (sort (select-non-precious)))
which in theory should be a fairly constant number (as we keep pruning it).
EA> And I really don't know why the precious entries are getting pruned.
EA> In a day or so, when I have more time, I'll test with a dummy registry,
EA> with max-entries set to 10 and 'creation-time added to the precious
EA> entries -- should make debugging easier.
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?
EA> Given the commentary in registry.el, I wonder if the whole pruning
EA> arrangement actually never quite got finished:
EA> ;; The user decides which fields are "precious", F2 for example. At
EA> ;; PRUNE TIME (when the :prune-function is called), the registry will
EA> ;; trim any entries without the F2 field until the size is :max-soft
EA> ;; or less. No entries with the F2 field will be removed at PRUNE
EA> ;; TIME.
EA> It looks like there's supposed to be a :prune-function slot on registry
EA> objects, and presumably the Gnus registry would have a prune function
EA> that worked more like what the comments above outline.
EA> I'd be happy to help work on this, with a little direction...
Assume it's broken. And feel free to propose or make bigger changes as
you see fit; there's little in Emacs to support this kind of database so
I wrote a lot of it from scratch. Sorry for the trouble.
Ted
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH] Two issues with the gnus-registry
2014-10-27 15:03 ` Ted Zlatanov
@ 2014-10-27 19:15 ` Eric Abrahamsen
2014-10-28 18:04 ` Eric Abrahamsen
2014-10-28 20:10 ` Ted Zlatanov
0 siblings, 2 replies; 24+ messages in thread
From: Eric Abrahamsen @ 2014-10-27 19:15 UTC (permalink / raw)
To: ding
Ted Zlatanov <tzz@lifelogs.com> writes:
> On Fri, 24 Oct 2014 12:04:25 -0700 Eric Abrahamsen <eric@ericabrahamsen.net> wrote:
>
> EA> I'm using the gnus-registry in Gnorb to keep track of correspondences
> EA> between Gnus messages and Org headings, using a key on the registry
> EA> entries called 'gnorb-ids. This is meant to be a "precious" key, ie
> EA> entries with this key are not pruned. So far so good:
>
> EA> (oref gnus-registry-db :precious) => (gnorb-ids mark)
>
> EA> But the entries still do get pruned! I've just had tracked entries start
> EA> disappearing on me, and realized that they're getting pruned from the
> EA> registry when I save it. There appear to be two problems here:
>
> EA> 1. The pruning doesn't start with the oldest entries first. The
> EA> docstring of `registry-prune' says it will, but looking over the code in
> EA> both registry.el and gnus-registry.el, I don't see how that would
> EA> happen. The entries aren't sorted as they're entered, nor does
> EA> `gnus-registry-save' pass a sortfun in to `registry-save'. In fact, the
> EA> entries I'm losing are the most recently-created ones.
>
> Huh. I could've sworn this worked. It must be my mistake.
>
> EA> 2. The bigger problem is that "precious" entries still seem to get
> EA> pruned. This is really difficult to edebug, because of the enormous
> EA> loops involved, and because any time the cursor passes the "db" variable
> EA> and tries to pretty-print it, Emacs runs out of memory (my registry has
> EA> about 18,000 entries in it). But something is still wrong here!
>
> Yeah, keeping the whole database in a big data structure is pretty bad.
> Plus I wrote that code in my "loop-happy" period. I'd like to use
> something better to store these records.
Better than a hash table? Dunno what that would be. But I suppose
precious entries could go in one table, and non-precious in another... I
don't think the "loop" macro itself is such a problem, but stepping
through the main prune or save functions does get cumbersome with large
data sets.
Eieio is supposed to make edebug use the object-print method when
displaying objects, but the line that would do that (eieio.el:895) is
currently commented out, I don't know why. If that were working,
registries could have their own object-print method, would make edebug
much more usable.
> EA> I added a sortfun to the `gnus-registry-save' call, but obviously it
> EA> slows the save process *way* the heck down. I'm not sure what else to
> EA> do, though, since (as far as I know) hash tables aren't guaranteed to
> EA> keep their sort order. Is that correct?
>
> Right. But you don't need to sort the whole database, only the
> non-precious entries. So it should be: (prune (sort (select-non-precious)))
> which in theory should be a fairly constant number (as we keep pruning it).
Right, makes sense. I started on a patch for the pruning process, but
realized I was a bit confused by the interaction between max-entries/:max-hard
and max-pruned-entries/:max-soft. Just to make sure I get it:
The :max-hard limit should *only* come into play when adding new
entries: they'll be rejected if the registry is already full.
The :max-soft limit should *only* come into play when pruning: we do our
best to prune down to :max-soft by deleting non-precious entries. If all
entries are precious, we accept a too-large registry.
Right now, the :prune-factor is used in conjunction with :max-hard. If
the above is correct, that's backwards: it should be used when pruning
with :max-soft.
Is all that correct?
> EA> And I really don't know why the precious entries are getting pruned.
> EA> In a day or so, when I have more time, I'll test with a dummy registry,
> EA> with max-entries set to 10 and 'creation-time added to the precious
> EA> entries -- should make debugging easier.
>
> 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.
> EA> Given the commentary in registry.el, I wonder if the whole pruning
> EA> arrangement actually never quite got finished:
>
> EA> ;; The user decides which fields are "precious", F2 for example. At
> EA> ;; PRUNE TIME (when the :prune-function is called), the registry will
> EA> ;; trim any entries without the F2 field until the size is :max-soft
> EA> ;; or less. No entries with the F2 field will be removed at PRUNE
> EA> ;; TIME.
>
> EA> It looks like there's supposed to be a :prune-function slot on registry
> EA> objects, and presumably the Gnus registry would have a prune function
> EA> that worked more like what the comments above outline.
>
> EA> I'd be happy to help work on this, with a little direction...
>
> Assume it's broken. And feel free to propose or make bigger changes as
> you see fit; there's little in Emacs to support this kind of database so
> I wrote a lot of it from scratch. Sorry for the trouble.
Not at all! It's worked great so far, and is impressive for something
done from scratch. My guess is not many people have had a need for the
precious functionality, so that hasn't been an issue. It's ideal
for use in Gnorb (tracking correspondences between messages and Org
headings), I just *really* need precious entries to hang around.
I also looked into subclassing the basic registry, but so much necessary
functionality (tracking message movement and summary-buffer scanning) is
only in gnus-registry, so that wasn't very realistic. At some point
(later) it might be worth considering making the gnus-registry an actual
subclass of the basic registry, and turning the various action
functions, etc, into methods.
That's getting ahead of things -- tests first.
Thanks,
Eric
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH] Two issues with the gnus-registry
2014-10-27 19:15 ` Eric Abrahamsen
@ 2014-10-28 18:04 ` Eric Abrahamsen
2014-11-07 23:56 ` Eric Abrahamsen
2014-10-28 20:10 ` Ted Zlatanov
1 sibling, 1 reply; 24+ messages in thread
From: Eric Abrahamsen @ 2014-10-28 18:04 UTC (permalink / raw)
To: ding
[-- Attachment #1: Type: text/plain, Size: 857 bytes --]
Eric Abrahamsen <eric@ericabrahamsen.net> writes:
> Ted Zlatanov <tzz@lifelogs.com> 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.
E
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Test-registry-pruning.patch --]
[-- Type: text/x-diff, Size: 3991 bytes --]
From 08491aff0b99e7e7ee04a0310174bc13aa0d1bcd Mon Sep 17 00:00:00 2001
From: Eric Abrahamsen <eric@ericabrahamsen.net>
Date: Tue, 28 Oct 2014 09:30:01 -0700
Subject: [PATCH] Test registry pruning
* lisp/registry.el (registry-prune): Change function to do what the
docstring says: return a count of total entries pruned.
* lisp/tests/gnustest-registry.el (gnustest-registry-usage-test):
Uncomment pruning part of test, and alter to correctly receive
number of entries pruned.
---
lisp/registry.el | 40 +++++++++++++++++++++-------------------
lisp/tests/gnustest-registry.el | 17 ++++++++---------
2 files changed, 29 insertions(+), 28 deletions(-)
diff --git a/lisp/registry.el b/lisp/registry.el
index dbc7b51..1d31d2b 100644
--- a/lisp/registry.el
+++ b/lisp/registry.el
@@ -319,25 +319,27 @@ 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))))
+ (let ((pruned 0))
+ (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)
+ (setq pruned (+ candidates-count pruned))))
+ pruned))
(defmethod registry-prune-soft-candidates ((db registry-db))
"Collects pruning candidates from the registry-db object THIS.
diff --git a/lisp/tests/gnustest-registry.el b/lisp/tests/gnustest-registry.el
index 174a0cb..7fa0498 100644
--- a/lisp/tests/gnustest-registry.el
+++ b/lisp/tests/gnustest-registry.el
@@ -101,15 +101,14 @@
(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 "Pruning")
+ (let* ((tokeep (registry-search db :member '((extra "more data"))))
+ (count (- n (length tokeep)))
+ (prune-count (registry-prune db)))
+ (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-persistence-test ()
--
2.1.2
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH] Two issues with the gnus-registry
2014-10-27 19:15 ` Eric Abrahamsen
2014-10-28 18:04 ` Eric Abrahamsen
@ 2014-10-28 20:10 ` Ted Zlatanov
1 sibling, 0 replies; 24+ messages in thread
From: Ted Zlatanov @ 2014-10-28 20:10 UTC (permalink / raw)
To: ding
On Mon, 27 Oct 2014 12:15:24 -0700 Eric Abrahamsen <eric@ericabrahamsen.net> wrote:
EA> Ted Zlatanov <tzz@lifelogs.com> writes:
>> Yeah, keeping the whole database in a big data structure is pretty bad.
>> Plus I wrote that code in my "loop-happy" period. I'd like to use
>> something better to store these records.
EA> Better than a hash table? Dunno what that would be.
An on-disk DB like LDBM, or a RDBMS like PostgreSQL, or a NoSQL store
like Redis, or a cloud database like DynamoDB. Or even an IMAP mailbox
with one message per key, since IMAP is quick to search by message ID.
I'd need to test them.
My personal preference for Fast Things I Know Will Work is LDBM. But the
IMAP mailbox could be very convenient and requires no C add-ons.
Ted
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH] Two issues with the gnus-registry
2014-10-28 18:04 ` Eric Abrahamsen
@ 2014-11-07 23:56 ` Eric Abrahamsen
2014-11-08 0:01 ` Eric Abrahamsen
0 siblings, 1 reply; 24+ messages in thread
From: Eric Abrahamsen @ 2014-11-07 23:56 UTC (permalink / raw)
To: ding
[-- Attachment #1: Type: text/plain, Size: 1572 bytes --]
Eric Abrahamsen <eric@ericabrahamsen.net> writes:
> Eric Abrahamsen <eric@ericabrahamsen.net> writes:
>
>> Ted Zlatanov <tzz@lifelogs.com> 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
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Preserve-precious-entries-when-pruning-registry.patch --]
[-- Type: text/x-diff, Size: 11201 bytes --]
From d19b46f0e3afdf6f2c8a30ab083b93d7d072718e Mon Sep 17 00:00:00 2001
From: Eric Abrahamsen <eric@ericabrahamsen.net>
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
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH] Two issues with the gnus-registry
2014-11-07 23:56 ` Eric Abrahamsen
@ 2014-11-08 0:01 ` Eric Abrahamsen
2014-11-08 8:39 ` Eric Abrahamsen
0 siblings, 1 reply; 24+ messages in thread
From: Eric Abrahamsen @ 2014-11-08 0:01 UTC (permalink / raw)
To: ding
[-- Attachment #1: Type: text/plain, Size: 2166 bytes --]
Eric Abrahamsen <eric@ericabrahamsen.net> writes:
> Eric Abrahamsen <eric@ericabrahamsen.net> writes:
>
>> Eric Abrahamsen <eric@ericabrahamsen.net> writes:
>>
>>> Ted Zlatanov <tzz@lifelogs.com> 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.
And of course seconds later I realized that the test didn't quite match
the logic of the function, here's a new version.
In essence, here's the logic of pruning, taken from the test:
(let* ((size (registry-size db))
(limit (min (oref db :max-hard)
(oref db :max-soft)))
(keepers (registry-search db :member '((extra "more data"))))
(desired-prune-count (- size (- limit (* 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))))
I hope that's what was actually intended! Seems to work okay, though.
Please test with byte-compiled files, otherwise it's brutally slow.
Eric
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Preserve-precious-entries-when-pruning-registry.patch --]
[-- Type: text/x-diff, Size: 11227 bytes --]
From 3a5ae90f585697fc384e2be4564d1b773f28e816 Mon Sep 17 00:00:00 2001
From: Eric Abrahamsen <eric@ericabrahamsen.net>
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 | 42 ++++++++++++------
texi/gnus.texi | 13 ++++--
4 files changed, 81 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..4b3fdd5 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,31 @@
(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))
+ (limit (min (oref db :max-hard)
+ (oref db :max-soft)))
+ (keepers (registry-search db :member '((extra "more data"))))
+ (desired-prune-count (- size (- limit (* 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
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH] Two issues with the gnus-registry
2014-11-08 0:01 ` Eric Abrahamsen
@ 2014-11-08 8:39 ` Eric Abrahamsen
2014-11-10 13:54 ` Ted Zlatanov
0 siblings, 1 reply; 24+ messages in thread
From: Eric Abrahamsen @ 2014-11-08 8:39 UTC (permalink / raw)
To: ding
Eric Abrahamsen <eric@ericabrahamsen.net> writes:
> Eric Abrahamsen <eric@ericabrahamsen.net> writes:
>
>> Eric Abrahamsen <eric@ericabrahamsen.net> writes:
>>
>>> Eric Abrahamsen <eric@ericabrahamsen.net> writes:
>>>
>>>> Ted Zlatanov <tzz@lifelogs.com> 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.
>
> And of course seconds later I realized that the test didn't quite match
> the logic of the function, here's a new version.
Nope, I still can't make this work -- I just don't get the relationship
between max-hard and max-soft. If max-soft is higher than max-hard, then
it will never get used at all. When the registry reaches max-hard in
size it will get pruned back, but pruned back to what size? To (*
max-soft prune-factor)? But that's likely already higher than
max-hard. To (* max-hard prune-factor)? In which case, what's the point
of max-soft?
If max-soft is less than max-hard, then it makes more sense -- when we
reach max-hard, we prune down to max-soft. But in that case the
prune-factor isn't really useful -- we might as well just prune directly
to max-soft, since pruning won't happen again until we're back up to
max-hard.
So unless I'm really missing something, my proposal is:
1. Only provide one limit: max-size.
2. Allow customization of prune-factor.
That seems like all the customization you'd need. Cap the size, and
provide a reasonable control of how often the registry gets pruned.
That would require a change in the object signature, which would mean
some extra handling code for "upgrading". But once we're doing that, we
could also take the opportunity to add :prune-function and
:sort-function slots on the base registry object, which would be
nice. We could even change the default store filename from its "eioio"
extension to "eieio". :)
Eric
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH] Two issues with the gnus-registry
2014-11-08 8:39 ` Eric Abrahamsen
@ 2014-11-10 13:54 ` Ted Zlatanov
2014-11-11 2:55 ` Eric Abrahamsen
2014-11-13 12:05 ` Eric Abrahamsen
0 siblings, 2 replies; 24+ messages in thread
From: Ted Zlatanov @ 2014-11-10 13:54 UTC (permalink / raw)
To: ding
On Sat, 08 Nov 2014 16:39:48 +0800 Eric Abrahamsen <eric@ericabrahamsen.net> wrote:
EA> So unless I'm really missing something, my proposal is:
EA> 1. Only provide one limit: max-size.
EA> 2. Allow customization of prune-factor.
EA> That seems like all the customization you'd need. Cap the size, and
EA> provide a reasonable control of how often the registry gets pruned.
OK. I am afraid I made the whole thing too complicated and it took
another set of eyes to recognize it. Thanks for that, and the fixes.
Please go ahead and apply any changes you have in mind. I am sure no one
is tuning soft/hard pruning. Simpler is definitely better.
EA> That would require a change in the object signature, which would mean
EA> some extra handling code for "upgrading". But once we're doing that, we
EA> could also take the opportunity to add :prune-function and
EA> :sort-function slots on the base registry object, which would be
EA> nice. We could even change the default store filename from its "eioio"
EA> extension to "eieio". :)
Yeah, I remember that bug report about the extension of the database
file, if we're upgrading anyway... It's a good chance to batch these
changes.
Ted
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH] Two issues with the gnus-registry
2014-11-10 13:54 ` Ted Zlatanov
@ 2014-11-11 2:55 ` Eric Abrahamsen
2014-11-13 12:05 ` Eric Abrahamsen
1 sibling, 0 replies; 24+ messages in thread
From: Eric Abrahamsen @ 2014-11-11 2:55 UTC (permalink / raw)
To: ding
Ted Zlatanov <tzz@lifelogs.com> writes:
> On Sat, 08 Nov 2014 16:39:48 +0800 Eric Abrahamsen <eric@ericabrahamsen.net> wrote:
>
> EA> So unless I'm really missing something, my proposal is:
>
> EA> 1. Only provide one limit: max-size.
> EA> 2. Allow customization of prune-factor.
>
> EA> That seems like all the customization you'd need. Cap the size, and
> EA> provide a reasonable control of how often the registry gets pruned.
>
> OK. I am afraid I made the whole thing too complicated and it took
> another set of eyes to recognize it. Thanks for that, and the fixes.
>
> Please go ahead and apply any changes you have in mind. I am sure no one
> is tuning soft/hard pruning. Simpler is definitely better.
Cool, mostly I'm glad I'm not missing something -- I discovered over
several days of staring at it that I'm pretty bad at mental math.
> EA> That would require a change in the object signature, which would mean
> EA> some extra handling code for "upgrading". But once we're doing that, we
> EA> could also take the opportunity to add :prune-function and
> EA> :sort-function slots on the base registry object, which would be
> EA> nice. We could even change the default store filename from its "eioio"
> EA> extension to "eieio". :)
>
> Yeah, I remember that bug report about the extension of the database
> file, if we're upgrading anyway... It's a good chance to batch these
> changes.
Okay. I guess this will require some (slightly unfortunate) one-off code
that will have to live in the codebase for quite some time, but I'll try
to make it as unobtrusive as possible, perhaps using advice. Maybe it
should even go in a separate file?
If there's something else you've been meaning to tweak regarding the
database format, let me know and let's get it done! You mentioned
wanting to provide alternate data storage options: I doubt I'll be of
much help there, but maybe we could leave a stub in place so that future
expansions don't require file format changes. It might even make sense
to separate the registry object and its :data store. The persistent
object, as it behaves right now, might look like:
(registry-db "Gnus Registry"
:file ".gnus.registry.eieio"
:max-size 50000
:data (file ".gnus.registry.dat"))
That would probably leave all the flexibility you'd need for future
extensions.
And... while I'm pulling things out of my rear end, I've also thought it
would be nice if the Gnus registry was actually a subclass of the
registry class. If all the message tracking stuff was done in methods,
it would be very easy for people to make their own special-purpose Gnus
registries.
Anyway, something to think about for the future.
Eric
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH] Two issues with the gnus-registry
2014-11-10 13:54 ` Ted Zlatanov
2014-11-11 2:55 ` Eric Abrahamsen
@ 2014-11-13 12:05 ` Eric Abrahamsen
2014-11-16 1:04 ` Dan Christensen
2014-12-18 10:07 ` Ted Zlatanov
1 sibling, 2 replies; 24+ messages in thread
From: Eric Abrahamsen @ 2014-11-13 12:05 UTC (permalink / raw)
To: ding
[-- Attachment #1: Type: text/plain, Size: 2399 bytes --]
Ted Zlatanov <tzz@lifelogs.com> writes:
> On Sat, 08 Nov 2014 16:39:48 +0800 Eric Abrahamsen <eric@ericabrahamsen.net> wrote:
>
> EA> So unless I'm really missing something, my proposal is:
>
> EA> 1. Only provide one limit: max-size.
> EA> 2. Allow customization of prune-factor.
>
> EA> That seems like all the customization you'd need. Cap the size, and
> EA> provide a reasonable control of how often the registry gets pruned.
>
> OK. I am afraid I made the whole thing too complicated and it took
> another set of eyes to recognize it. Thanks for that, and the fixes.
>
> Please go ahead and apply any changes you have in mind. I am sure no one
> is tuning soft/hard pruning. Simpler is definitely better.
>
> EA> That would require a change in the object signature, which would mean
> EA> some extra handling code for "upgrading". But once we're doing that, we
> EA> could also take the opportunity to add :prune-function and
> EA> :sort-function slots on the base registry object, which would be
> EA> nice. We could even change the default store filename from its "eioio"
> EA> extension to "eieio". :)
>
> Yeah, I remember that bug report about the extension of the database
> file, if we're upgrading anyway... It's a good chance to batch these
> changes.
Okay, here is Stab the First.
Most of what's happening should be evident from commit messages and code
comments. There are three patches:
1. The big one changes the database structure, combining :max-hard and
:max-soft into :max-size, and reworking the pruning routine to only make
a single pass. This also includes a new initialize-instance method for
upgrading from older versions, and the extraction of the database
version into a new defvar: see the comments on top of
`registry-db-version'.
2. The second patch changes the default filename to a eieio extension,
which necessitated more code than I thought -- `gnus-registry-read' is
split into two functions. I hope this isn't too crufty.
3. Number three introduces sorting into the pruning process. I
considered adding a sorting function as a slot on the registry-db
objects, but couldn't immediately think of why that was really
necessary. Anyway, there's a new option
`gnus-registry-default-sorting-function', which defaults to a function
that sorts by 'creation-time.
Tests and documentation all around.
Presumably there are bugs! I hope people will test.
Eric
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0003-Sort-registry-entries-when-pruning.patch --]
[-- Type: text/x-diff, Size: 8104 bytes --]
From 9e4e8993aaef6a861b2237e0509eeee02089916f Mon Sep 17 00:00:00 2001
From: Eric Abrahamsen <eric@ericabrahamsen.net>
Date: Wed, 12 Nov 2014 13:49:36 +0800
Subject: [PATCH 3/4] Sort registry entries when pruning
* lisp/registry.el (registry-prune): Accept and use a sort-function
argument.
* lisp/gnus-registry.el (gnus-registry-default-sort-function): New
customization option pointing at a sort function to use when
pruning.
(gnus-registry-sort-by-creation-time): Provide a default function
for the above option.
(gnus-registry-save, gnus-registry-insert): Use sort function.
* lisp/tests/gnustest-registry.el (gnustest-registry-sort-function):
Default sort function for testing.
(gnustest-registry-pruning-sort-test): New test for sorting.
* text/gnus.texi: Document sorting options.
---
lisp/gnus-registry.el | 25 +++++++++++++++++++++++--
lisp/registry.el | 21 +++++++++++++++------
lisp/tests/gnustest-registry.el | 26 ++++++++++++++++++++++++++
texi/gnus.texi | 9 +++++++++
4 files changed, 73 insertions(+), 8 deletions(-)
diff --git a/lisp/gnus-registry.el b/lisp/gnus-registry.el
index 0f3ea23..92f8f04 100644
--- a/lisp/gnus-registry.el
+++ b/lisp/gnus-registry.el
@@ -257,6 +257,25 @@ entries. The pruning process is constrained by the presence of
:group 'gnus-registry
:type 'float)
+(defcustom gnus-registry-default-sort-function
+ #'gnus-registry-sort-by-creation-time
+ "Sort function to use when pruning the registry.
+
+Entries which sort to the front of the list will be pruned
+first.
+
+This can slow pruning down. Set to nil to perform no sorting."
+ :version "24.4"
+ :group 'gnus-registry
+ :type 'symbol)
+
+(defun gnus-registry-sort-by-creation-time (l r)
+ "Sort older entries to front of list."
+ ;; Pruning starts from the front of the list.
+ (time-less-p
+ (cadr (assq 'creation-time r))
+ (cadr (assq 'creation-time l))))
+
(defun gnus-registry-fixup-registry (db)
(when db
(let ((old (oref db :tracked)))
@@ -349,7 +368,8 @@ This is not required after changing `gnus-registry-cache-file'."
(db (or db gnus-registry-db)))
(gnus-message 5 "Saving Gnus registry (%d entries) to %s..."
(registry-size db) file)
- (registry-prune db)
+ (registry-prune
+ db gnus-registry-default-sort-function)
;; TODO: call (gnus-string-remove-all-properties v) on all elements?
(eieio-persistent-save db file)
(gnus-message 5 "Saving Gnus registry (size %d) to %s...done"
@@ -1056,7 +1076,8 @@ only the last one's marks are returned."
"Just like `registry-insert' but tries to prune on error."
(when (registry-full db)
(message "Trying to prune the registry because it's full")
- (registry-prune db))
+ (registry-prune
+ db gnus-registry-default-sort-function))
(registry-insert db id entry)
entry)
diff --git a/lisp/registry.el b/lisp/registry.el
index 86e1ee3..d949e7a 100644
--- a/lisp/registry.el
+++ b/lisp/registry.el
@@ -334,7 +334,7 @@ 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))
+(defmethod registry-prune ((db registry-db) &optional sortfunc)
"Prunes the registry-db object DB.
Attempts to prune the number of entries down to \(*
@@ -343,6 +343,9 @@ 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.
+Entries to be pruned are first sorted using SORTFUNC. Entries
+from the front of the list are deleted first.
+
Returns the number of deleted entries."
(let ((size (registry-size db))
(target-size (- (oref db :max-size)
@@ -352,15 +355,17 @@ Returns the number of deleted entries."
(if (> size target-size)
(progn
(setq candidates
- (registry-collect-prune-candidates db (- size target-size)))
+ (registry-collect-prune-candidates
+ db (- size target-size) sortfunc))
(length (registry-delete db candidates nil)))
0)))
-(defmethod registry-collect-prune-candidates ((db registry-db) limit)
+(defmethod registry-collect-prune-candidates ((db registry-db) limit sortfunc)
"Collects pruning candidates from the registry-db object DB.
Proposes only entries without the :precious keys, and attempts to
-return LIMIT such candidates."
+return LIMIT such candidates. If SORTFUNC is provided, sort
+entries first and return candidates from beginning of list."
(let* ((precious (oref db :precious))
(precious-p (lambda (entry-key)
(cdr (memq (car entry-key) precious))))
@@ -368,8 +373,12 @@ return LIMIT such 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))))
+ collect (cons k v))))
+ ;; We want the full entries for sorting, but should only return a
+ ;; list of entry keys.
+ (when sortfunc
+ (setq candidates (sort candidates sortfunc)))
+ (delq nil (cl-subseq (mapcar #'car candidates) 0 limit))))
(provide 'registry)
;;; registry.el ends here
diff --git a/lisp/tests/gnustest-registry.el b/lisp/tests/gnustest-registry.el
index 5ca0cc0..e6ac8db 100644
--- a/lisp/tests/gnustest-registry.el
+++ b/lisp/tests/gnustest-registry.el
@@ -52,6 +52,11 @@
(should-not (registry--match :member entry '((hello)))))
(message "Done with matching testing."))
+(defun gnustest-registry-sort-function (l r)
+ "Sort lower values of sort-field earlier."
+ (< (cadr (assq 'sort-field l))
+ (cadr (assq 'sort-field r))))
+
(defun gnustest-registry-make-testable-db (n &optional prune-factor name file)
(let* ((db (registry-db
(or name "Testing")
@@ -66,6 +71,7 @@
(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")))
+ (sort-field ,(- n i))
(groups ,(number-to-string i)))))
db))
@@ -126,6 +132,26 @@
:prefix "Error: ")
(should (= expected-prune-count actual-prune-count)))))))
+(ert-deftest gnustest-registry-pruning-sort-test ()
+ "Check that entries are sorted properly before pruning."
+ (let ((db (gnustest-registry-make-testable-db 10 0.4))
+ ;; These entries have the highest 'sort-field values. Pruning
+ ;; sorts by lowest values first, then prunes from the front of
+ ;; the list, so these entries survive
+ (expected-survivors '(5 6 7 8 9 0))
+ actual-survivors disjunct)
+ (registry-prune
+ db #'gnustest-registry-sort-function)
+ (maphash (lambda (k v) (push k actual-survivors))
+ (oref db :data))
+ (setq disjunct (cl-set-exclusive-or
+ expected-survivors
+ actual-survivors))
+ (ert-info
+ ((format "Incorrect pruning: %s" disjunct)
+ :prefix "Error: ")
+ (should (null disjunct)))))
+
(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 ca7ddfe..9255ed9 100644
--- a/texi/gnus.texi
+++ b/texi/gnus.texi
@@ -25958,6 +25958,15 @@ cut back to 45000 entries. Entries with keys marked as precious will
not be pruned.
@end defvar
+@defvar gnus-registry-default-sort-function
+This option specifies how registry entries are sorted during pruning.
+If a function is given, it should sort least valuable entries first,
+as pruning starts from the beginning of the list. The default value
+is @code{gnus-registry-sort-by-creation-time}, which proposes the
+oldest entries for pruning. Set to nil to perform no sorting, which
+will speed up the pruning process.
+@end defvar
+
@defvar gnus-registry-cache-file
The file where the registry will be stored between Gnus sessions. By
default the file name is @code{.gnus.registry.eieio} in the same
--
2.1.3
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: 0002-Change-default-registry-filename-extension.patch --]
[-- Type: text/x-diff, Size: 5022 bytes --]
From 2ef0a99726eaeaab7831fa2cd7af0e3dd12c5e72 Mon Sep 17 00:00:00 2001
From: Eric Abrahamsen <eric@ericabrahamsen.net>
Date: Wed, 12 Nov 2014 13:04:24 +0800
Subject: [PATCH 2/4] Change default registry filename extension
* lisp/gnus-registry.el (gnus-registry-cache-file): Change default
filename extension to "eieio".
(gnus-registry-read): New function, split out from
`gnus-registry-load', that does the actual object reading.
(gnus-registry-load): Refactor to call new function
`gnus-registry-read', also add condition case handler to check for
old filename extension
---
lisp/gnus-registry.el | 52 ++++++++++++++++++++++++++++++++++-----------------
texi/gnus.texi | 2 +-
2 files changed, 36 insertions(+), 18 deletions(-)
diff --git a/lisp/gnus-registry.el b/lisp/gnus-registry.el
index d3f6546..0f3ea23 100644
--- a/lisp/gnus-registry.el
+++ b/lisp/gnus-registry.el
@@ -232,7 +232,7 @@ the Bit Bucket."
(defcustom gnus-registry-cache-file
(nnheader-concat
(or gnus-dribble-directory gnus-home-directory "~/")
- ".gnus.registry.eioio")
+ ".gnus.registry.eieio")
"File where the Gnus registry will be stored."
:group 'gnus-registry
:type 'file)
@@ -301,22 +301,27 @@ This is not required after changing `gnus-registry-cache-file'."
(gnus-message 4 "Remaking the Gnus registry")
(setq gnus-registry-db (gnus-registry-make-db))))
-(defun gnus-registry-read ()
- "Read the registry cache file."
+(defun gnus-registry-load ()
+ "Load the registry from the cache file."
(interactive)
(let ((file gnus-registry-cache-file))
(condition-case nil
- (progn
- (gnus-message 5 "Reading Gnus registry from %s..." file)
- (setq gnus-registry-db
- (gnus-registry-fixup-registry
- (condition-case nil
- (with-no-warnings
- (eieio-persistent-read file 'registry-db))
- ;; Older EIEIO versions do not check the class name.
- ('wrong-number-of-arguments
- (eieio-persistent-read file)))))
- (gnus-message 5 "Reading Gnus registry from %s...done" file))
+ (gnus-registry-read file)
+ (file-error
+ ;; Fix previous mis-naming of the registry file.
+ (let ((old-file-name
+ (concat (file-name-sans-extension
+ gnus-registry-cache-file)
+ ".eioio")))
+ (if (and (file-exists-p old-file-name)
+ (yes-or-no-p
+ (format "Rename registry file from %s to %s? "
+ old-file-name file)))
+ (progn
+ (gnus-registry-read old-file-name)
+ (oset gnus-registry-db :file file)
+ (gnus-message 1 "Registry filename changed to %s" file))
+ (gnus-registry-remake-db t))))
(error
(gnus-message
1
@@ -324,6 +329,19 @@ This is not required after changing `gnus-registry-cache-file'."
file)
(gnus-registry-remake-db t)))))
+(defun gnus-registry-read (file)
+ "Do the actual reading of the registry persistence file."
+ (gnus-message 5 "Reading Gnus registry from %s..." file)
+ (setq gnus-registry-db
+ (gnus-registry-fixup-registry
+ (condition-case nil
+ (with-no-warnings
+ (eieio-persistent-read file 'registry-db))
+ ;; Older EIEIO versions do not check the class name.
+ ('wrong-number-of-arguments
+ (eieio-persistent-read file)))))
+ (gnus-message 5 "Reading Gnus registry from %s...done" file))
+
(defun gnus-registry-save (&optional file db)
"Save the registry cache file."
(interactive)
@@ -1096,7 +1114,7 @@ only the last one's marks are returned."
(gnus-message 5 "Initializing the registry")
(gnus-registry-install-hooks)
(gnus-registry-install-shortcuts)
- (gnus-registry-read))
+ (gnus-registry-load))
;; FIXME: Why autoload this function?
;;;###autoload
@@ -1110,7 +1128,7 @@ only the last one's marks are returned."
(add-hook 'nnmail-spool-hook 'gnus-registry-spool-action)
(add-hook 'gnus-save-newsrc-hook 'gnus-registry-save)
- (add-hook 'gnus-read-newsrc-el-hook 'gnus-registry-read)
+ (add-hook 'gnus-read-newsrc-el-hook 'gnus-registry-load)
(add-hook 'gnus-summary-prepare-hook 'gnus-registry-register-message-ids))
@@ -1123,7 +1141,7 @@ only the last one's marks are returned."
(remove-hook 'nnmail-spool-hook 'gnus-registry-spool-action)
(remove-hook 'gnus-save-newsrc-hook 'gnus-registry-save)
- (remove-hook 'gnus-read-newsrc-el-hook 'gnus-registry-read)
+ (remove-hook 'gnus-read-newsrc-el-hook 'gnus-registry-load)
(remove-hook 'gnus-summary-prepare-hook 'gnus-registry-register-message-ids)
(setq gnus-registry-enabled nil))
diff --git a/texi/gnus.texi b/texi/gnus.texi
index 082adad..ca7ddfe 100644
--- a/texi/gnus.texi
+++ b/texi/gnus.texi
@@ -25960,7 +25960,7 @@ not be pruned.
@defvar gnus-registry-cache-file
The file where the registry will be stored between Gnus sessions. By
-default the file name is @code{.gnus.registry.eioio} in the same
+default the file name is @code{.gnus.registry.eieio} in the same
directory as your @code{.newsrc.eld}.
@end defvar
--
2.1.3
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #4: 0001-Alter-registry-database-to-fix-pruning-issues.patch --]
[-- Type: text/x-diff, Size: 17827 bytes --]
From 7da7dc2222137b03ff8f54e61ac0c519b7316f0c Mon Sep 17 00:00:00 2001
From: Eric Abrahamsen <eric@ericabrahamsen.net>
Date: Wed, 12 Nov 2014 12:59:45 +0800
Subject: [PATCH 1/4] Alter registry database to fix pruning issues
* lisp/registry.el (registry-db): Consolidate the :max-hard and
:max-soft slots into a :max-size slot.
(registry-db-version): New defvar, holding database version number.
(registry-prune): Use :max-size slot
(registry-collect-prune-candidates): New function for finding
non-precious pruning candidates
(registry-prune-hard-candidates, registry-prune-soft-candidates):
Remove functions
(initialize-instance): Perform registry version upgrade
* lisp/gnus-registry.el (gnus-registry-prune-factor): New custom
option
(gnus-registry-max-pruned-entries): Remove option
* lisp/tests/gnustest-registry.el (gnustest-registry-make-testable-db):
Alter for new object signature
(gnustest-registry-pruning-test): New pruning test
* texi/gnus.texi (Gnus Registry Setup): Document changes
---
lisp/gnus-registry.el | 30 ++++----
lisp/registry.el | 150 +++++++++++++++++++++-------------------
lisp/tests/gnustest-registry.el | 50 +++++++++-----
texi/gnus.texi | 16 +++--
4 files changed, 140 insertions(+), 106 deletions(-)
diff --git a/lisp/gnus-registry.el b/lisp/gnus-registry.el
index f3b81f7..d3f6546 100644
--- a/lisp/gnus-registry.el
+++ b/lisp/gnus-registry.el
@@ -176,6 +176,7 @@ nnmairix groups are specifically excluded because they are ephemeral."
(make-obsolete-variable 'gnus-registry-max-track-groups nil "23.4")
(make-obsolete-variable 'gnus-registry-entry-caching nil "23.4")
(make-obsolete-variable 'gnus-registry-trim-articles-without-groups nil "23.4")
+(make-obsolete-variable 'gnus-registry-max-pruned-entries nil "24.4")
(defcustom gnus-registry-track-extra '(subject sender recipient)
"Whether the registry should track extra data about a message.
@@ -242,12 +243,19 @@ the Bit Bucket."
:type '(radio (const :format "Unlimited " nil)
(integer :format "Maximum number: %v")))
-(defcustom gnus-registry-max-pruned-entries nil
- "Maximum number of pruned entries in the registry, nil for unlimited."
- :version "24.1"
+(defcustom gnus-registry-prune-factor 0.1
+ "When pruning, try to prune back to this factor less than the maximum size.
+
+In order to prevent constant pruning, we prune back to a number
+somewhat less than the maximum size. This option controls
+exactly how much less. For example, given a maximum size of
+50000 and a prune factor of 0.1, the pruning process will try to
+cut the registry back to \(- 50000 \(* 50000 0.1\)\) -> 45000
+entries. The pruning process is constrained by the presence of
+\"precious\" entries."
+ :version "24.4"
:group 'gnus-registry
- :type '(radio (const :format "Unlimited " nil)
- (integer :format "Maximum number: %v")))
+ :type 'float)
(defun gnus-registry-fixup-registry (db)
(when db
@@ -255,14 +263,12 @@ the Bit Bucket."
(oset db :precious
(append gnus-registry-extra-entries-precious
'()))
- (oset db :max-hard
+ (oset db :max-size
(or gnus-registry-max-entries
most-positive-fixnum))
(oset db :prune-factor
- 0.1)
- (oset db :max-soft
- (or gnus-registry-max-pruned-entries
- most-positive-fixnum))
+ (or gnus-registry-prune-factor
+ 0.1))
(oset db :tracked
(append gnus-registry-track-extra
'(mark group keyword)))
@@ -278,8 +284,8 @@ the Bit Bucket."
"Gnus Registry"
:file (or file gnus-registry-cache-file)
;; these parameters are set in `gnus-registry-fixup-registry'
- :max-hard most-positive-fixnum
- :max-soft most-positive-fixnum
+ :max-size most-positive-fixnum
+ :version registry-db-version
:precious nil
:tracked nil)))
diff --git a/lisp/registry.el b/lisp/registry.el
index dbc7b51..86e1ee3 100644
--- a/lisp/registry.el
+++ b/lisp/registry.el
@@ -25,11 +25,11 @@
;; This library provides a general-purpose EIEIO-based registry
;; database with persistence, initialized with these fields:
-;; version: a float, 0.1 currently (don't change it)
+;; version: a float
-;; max-hard: an integer, default 5000000
+;; max-size: an integer, default 50000
-;; max-soft: an integer, default 50000
+;; prune-factor: a float between 0 and 1, default 0.1
;; precious: a list of symbols
@@ -57,14 +57,15 @@
;; 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.
+;; The user decides which fields are "precious", F2 for example. When
+;; the registry is pruned, any entries without the F2 field will be
+;; removed until the size is :max-size * :prune-factor _less_ than the
+;; maximum database size. No entries with the F2 field will be removed
+;; at PRUNE TIME, which means it may not be possible to prune back all
+;; the way to the target size.
-;; 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
+;; When an entry is inserted, the registry will reject new entries if
+;; they bring it over the :max-size limit, even if they have the F2
;; field.
;; The user decides which fields are "tracked", F1 for example. Any
@@ -94,28 +95,32 @@
(error
"eieio not found in `load-path' or gnus-fallback-lib/ directory.")))
+;; The version number needs to be kept outside of the class definition
+;; itself. The persistent-save process does *not* write to file any
+;; slot values that are equal to the default :initform value. If a
+;; database object is at the most recent version, therefore, its
+;; version number will not be written to file. That makes it
+;; difficult to know when a database needs to be upgraded.
+(defvar registry-db-version 0.2
+ "The current version of the registry format.")
+
(defclass registry-db (eieio-persistent)
((version :initarg :version
- :initform 0.1
- :type float
- :custom float
+ :initform nil
+ :type (or null float)
:documentation "The registry version.")
- (max-hard :initarg :max-hard
- :initform 5000000
- :type integer
- :custom integer
- :documentation "Never accept more than this many elements.")
- (max-soft :initarg :max-soft
- :initform 50000
+ (max-size :initarg :max-size
+ :initform most-positive-fixnum
:type integer
:custom integer
- :documentation "Prune as much as possible to get to this size.")
+ :documentation "The maximum number of registry entries.")
(prune-factor
:initarg :prune-factor
:initform 0.1
:type float
:custom float
- :documentation "At the max-hard limit, prune size * this entries.")
+ :documentation "Prune to \(:max-size * :prune-factor\) less
+ than the :max-size limit. Should be a float between 0 and 1.")
(tracked :initarg :tracked
:initform nil
:type t
@@ -131,6 +136,23 @@
:type hash-table
:documentation "The data hashtable.")))
+(defmethod initialize-instance :BEFORE ((this registry-db) slots)
+ "Check whether a registry object needs to be upgraded."
+ ;; Hardcoded upgrade routines. Version 0.1 to 0.2 requires the
+ ;; :max-soft slot to disappear, and the :max-hard slot to be renamed
+ ;; :max-size.
+ (let ((current-version
+ (and (plist-member slots :version)
+ (plist-get slots :version))))
+ (when (or (null current-version)
+ (eql current-version 0.1))
+ (setq slots
+ (plist-put slots :max-size (plist-get slots :max-hard)))
+ (setq slots
+ (plist-put slots :version registry-db-version))
+ (cl-remf slots :max-hard)
+ (cl-remf slots :max-soft))))
+
(defmethod initialize-instance :AFTER ((this registry-db) slots)
"Set value of data slot of THIS after initialization."
(with-slots (data tracker) this
@@ -267,7 +289,7 @@ This is the key count of the :data slot."
(defmethod registry-full ((db registry-db))
"Checks if registry-db THIS is full."
(>= (registry-size db)
- (oref db :max-hard)))
+ (oref db :max-size)))
(defmethod registry-insert ((db registry-db) key entry)
"Insert ENTRY under KEY into the registry-db THIS.
@@ -279,7 +301,7 @@ Errors out if the key exists already."
(assert (not (registry-full db))
nil
- "registry max-hard size limit reached")
+ "registry max-size limit reached")
;; store the entry
(puthash key entry (oref db :data))
@@ -312,58 +334,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 \(*
+:max-size :prune-factor\) less than the max-size 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))
+ (target-size (- (oref db :max-size)
+ (* (oref db :max-size)
+ (oref db :prune-factor))))
+ candidates)
+ (if (> size target-size)
+ (progn
+ (setq candidates
+ (registry-collect-prune-candidates db (- size target-size)))
+ (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..5ca0cc0 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 prune-factor name file)
(let* ((db (registry-db
(or name "Testing")
:file (or file "unused")
- :max-hard n
- :max-soft 0 ; keep nothing not precious
+ :max-size n
+ :prune-factor (or prune-factor 0.1)
: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,22 +101,36 @@
(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 0.1)
+ ;; Pruning limited by precious entries.
+ (gnustest-registry-make-testable-db 10 0.6))))
+ (dolist (db dbs)
+ (message "Pruning")
+ (let* ((size (registry-size db))
+ (limit (- (oref db :max-size)
+ (* (oref db :max-size)
+ (oref db :prune-factor))))
+ (keepers (registry-search db :member '((extra "more data"))))
+ (expected-prune-count (min (- size (length keepers))
+ (- size limit)))
+ (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-"))
(name "persistence tester")
- (db (gnustest-registry-make-testable-db n name tempfile))
+ (db (gnustest-registry-make-testable-db n nil name tempfile))
size back)
(message "Saving to %s" tempfile)
(eieio-persistent-save db)
@@ -205,8 +219,8 @@
(should (= (registry-size back) n))
(should (= (registry-size back) (registry-size db)))
(delete-file tempfile)
- (message "Pruning Gnus registry to 0 by setting :max-soft")
- (oset db :max-soft 0)
+ (message "Pruning Gnus registry to 0 by setting :max-size")
+ (oset db :max-size 0)
(registry-prune db)
(should (= (registry-size db) 0)))
(message "Done with Gnus registry usage testing."))
diff --git a/texi/gnus.texi b/texi/gnus.texi
index 6f47786..082adad 100644
--- a/texi/gnus.texi
+++ b/texi/gnus.texi
@@ -25942,12 +25942,20 @@ 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.
+@defvar gnus-registry-prune-factor
+This option (a float between 0 and 1) controls how much the registry
+is cut back during pruning. In order to prevent constant pruning, the
+registry will be pruned back to less than
+@code{gnus-registry-max-entries}. This option controls exactly how
+much less: the target is calculated as the maximum number of entries
+minus the maximum number times this factor. The default is 0.1:
+i.e. if your registry is limited to 50000 entries, pruning will try to
+cut back to 45000 entries. Entries with keys marked as precious will
+not be pruned.
@end defvar
@defvar gnus-registry-cache-file
--
2.1.3
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH] Two issues with the gnus-registry
2014-11-13 12:05 ` Eric Abrahamsen
@ 2014-11-16 1:04 ` Dan Christensen
2014-11-16 3:24 ` Eric Abrahamsen
2014-12-18 10:07 ` Ted Zlatanov
1 sibling, 1 reply; 24+ messages in thread
From: Dan Christensen @ 2014-11-16 1:04 UTC (permalink / raw)
To: ding
Eric Abrahamsen <eric@ericabrahamsen.net> writes:
> 1. The big one changes the database structure, combining :max-hard and
> :max-soft into :max-size
Does this mean that precious entries will *never* get pruned?
Will that be a problem?
Also, if precious entries count towards the total, then once
you have as many precious entries as the total size, no other
entries will ever be kept...
What are precious entries currently used for?
Dan
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH] Two issues with the gnus-registry
2014-11-16 1:04 ` Dan Christensen
@ 2014-11-16 3:24 ` Eric Abrahamsen
0 siblings, 0 replies; 24+ messages in thread
From: Eric Abrahamsen @ 2014-11-16 3:24 UTC (permalink / raw)
To: ding
Dan Christensen <jdc@uwo.ca> writes:
> Eric Abrahamsen <eric@ericabrahamsen.net> writes:
>
>> 1. The big one changes the database structure, combining :max-hard and
>> :max-soft into :max-size
>
> Does this mean that precious entries will *never* get pruned?
> Will that be a problem?
>
> Also, if precious entries count towards the total, then once
> you have as many precious entries as the total size, no other
> entries will ever be kept...
>
> What are precious entries currently used for?
Yup, the whole point of precious entries is that you don't want them to
be pruned. This was how the registry was supposed to operate to begin
with, it just wasn't working correctly.
And yes, the registry refuses to add new entries over the total size,
precious or not. When it hits the limit, it tries to prune. If all the
entries in the registry are precious, it will lock up. I'd be mighty
surprised if anyone ever gets to this situation.
*No* entries are made precious in the default registry setup, so my
changes here won't affect anyone who hasn't gone and used preciousness
for their own purposes. I think that's why no one has run into this
problem before.
Users and package authors who make heavy use of this feature should
probably have a secondary "pruning" routine on hand that removes the
precious keys from entries that no longer need them. The registry could
probably do a little more to signal that it was full of precious
entries, so those routines could be run automatically.
I'd be happy to discuss how all this should work, and make any changes
that Ted is okay with. I think the registry is a great idea, and one
that's currently underutilized.
Eric
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH] Two issues with the gnus-registry
2014-11-13 12:05 ` Eric Abrahamsen
2014-11-16 1:04 ` Dan Christensen
@ 2014-12-18 10:07 ` Ted Zlatanov
2014-12-18 15:00 ` Eric Abrahamsen
1 sibling, 1 reply; 24+ messages in thread
From: Ted Zlatanov @ 2014-12-18 10:07 UTC (permalink / raw)
To: ding
On Thu, 13 Nov 2014 20:05:38 +0800 Eric Abrahamsen <eric@ericabrahamsen.net> wrote:
EA> Okay, here is Stab the First.
Sorry for the delay (poke me next time, please!). Applied as a single
patch because there were many related parts.
Ted
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH] Two issues with the gnus-registry
2014-12-18 10:07 ` Ted Zlatanov
@ 2014-12-18 15:00 ` Eric Abrahamsen
2014-12-18 15:09 ` Eric Abrahamsen
0 siblings, 1 reply; 24+ messages in thread
From: Eric Abrahamsen @ 2014-12-18 15:00 UTC (permalink / raw)
To: ding
Ted Zlatanov <tzz@lifelogs.com> writes:
> On Thu, 13 Nov 2014 20:05:38 +0800 Eric Abrahamsen <eric@ericabrahamsen.net> wrote:
>
> EA> Okay, here is Stab the First.
>
> Sorry for the delay (poke me next time, please!). Applied as a single
> patch because there were many related parts.
No worries, it seemed like the sort of thing that would be fairly dreary
to review; there wasn't much rush. And a single patch is fine, of
course.
Hope nothing breaks!
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH] Two issues with the gnus-registry
2014-12-18 15:00 ` Eric Abrahamsen
@ 2014-12-18 15:09 ` Eric Abrahamsen
2014-12-19 0:44 ` Katsumi Yamaoka
2014-12-19 1:30 ` [PATCH] Two issues with the gnus-registry Ted Zlatanov
0 siblings, 2 replies; 24+ messages in thread
From: Eric Abrahamsen @ 2014-12-18 15:09 UTC (permalink / raw)
To: ding
Eric Abrahamsen <eric@ericabrahamsen.net> writes:
> Ted Zlatanov <tzz@lifelogs.com> writes:
>
>> On Thu, 13 Nov 2014 20:05:38 +0800 Eric Abrahamsen <eric@ericabrahamsen.net> wrote:
>>
>> EA> Okay, here is Stab the First.
>>
>> Sorry for the delay (poke me next time, please!). Applied as a single
>> patch because there were many related parts.
>
> No worries, it seemed like the sort of thing that would be fairly dreary
> to review; there wasn't much rush. And a single patch is fine, of
> course.
>
> Hope nothing breaks!
Looks like something already did! I don't know why though. Compiling
gnus on its own works fine, but emacs compilation does indeed break with
the "Invalid slot type: max-size integer most-positive-fixnum".
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH] Two issues with the gnus-registry
2014-12-18 15:09 ` Eric Abrahamsen
@ 2014-12-19 0:44 ` Katsumi Yamaoka
2014-12-19 2:08 ` Eric Abrahamsen
2014-12-20 3:09 ` Ted Zlatanov
2014-12-19 1:30 ` [PATCH] Two issues with the gnus-registry Ted Zlatanov
1 sibling, 2 replies; 24+ messages in thread
From: Katsumi Yamaoka @ 2014-12-19 0:44 UTC (permalink / raw)
To: Eric Abrahamsen; +Cc: ding
On Thu, 18 Dec 2014 23:09:41 +0800, Eric Abrahamsen wrote:
> Looks like something already did! I don't know why though. Compiling
> gnus on its own works fine, but emacs compilation does indeed break with
> the "Invalid slot type: max-size integer most-positive-fixnum".
This should have been fixed, thanks to Paul Eggert.
But the Gnus buildbot[1] still complains there are glitches for
compiling with old Emacsen (including XEmacsen). Maybe it is
due to some new cl macros. Here is a compilation log I got in
my 32-bit Cygwin environment:
In registry-db::registry-collect-prune-candidates:
registry.el:375:31:Warning: reference to free variable `for'
registry.el:375:35:Warning: reference to free variable `k'
registry.el:375:37:Warning: reference to free variable `being'
registry.el:375:43:Warning: reference to free variable `the'
registry.el:375:47:Warning: reference to free variable `hash-keys'
registry.el:375:57:Warning: reference to free variable `of'
registry.el:376:31:Warning: reference to free variable `using'
registry.el:376:50:Warning: reference to free variable `v'
registry.el:377:31:Warning: reference to free variable `when'
registry.el:378:31:Warning: reference to free variable `collect'
In end of data:
registry.el:387:1:Warning: the following functions are not known to be
defined: cl-remf, cl-loop, hash-values, cl-subseq
Wrote /Work/gnus/lisp/registry.elc
This is what Emacs 23.2 said. Logs that XEmacs 21.4 and 21.5
issued are similar.
[1] http://www.randomsample.de:4456/waterfall
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH] Two issues with the gnus-registry
2014-12-18 15:09 ` Eric Abrahamsen
2014-12-19 0:44 ` Katsumi Yamaoka
@ 2014-12-19 1:30 ` Ted Zlatanov
1 sibling, 0 replies; 24+ messages in thread
From: Ted Zlatanov @ 2014-12-19 1:30 UTC (permalink / raw)
To: ding
On Thu, 18 Dec 2014 23:09:41 +0800 Eric Abrahamsen <eric@ericabrahamsen.net> wrote:
EA> Looks like something already did! I don't know why though. Compiling
EA> gnus on its own works fine, but emacs compilation does indeed break with
EA> the "Invalid slot type: max-size integer most-positive-fixnum".
`defclass' didn't expand the maxint value. I didn't catch it either,
sorry.
It was properly fixed by Paul Eggert, take a look.
Ted
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH] Two issues with the gnus-registry
2014-12-19 0:44 ` Katsumi Yamaoka
@ 2014-12-19 2:08 ` Eric Abrahamsen
2014-12-20 3:09 ` Ted Zlatanov
1 sibling, 0 replies; 24+ messages in thread
From: Eric Abrahamsen @ 2014-12-19 2:08 UTC (permalink / raw)
To: ding
Katsumi Yamaoka <yamaoka@jpl.org> writes:
> On Thu, 18 Dec 2014 23:09:41 +0800, Eric Abrahamsen wrote:
>> Looks like something already did! I don't know why though. Compiling
>> gnus on its own works fine, but emacs compilation does indeed break with
>> the "Invalid slot type: max-size integer most-positive-fixnum".
>
> This should have been fixed, thanks to Paul Eggert.
That's a relief it wasn't more complicated. I'm still not clear why I
was able to compile the file from within Emacs with no errors.
> But the Gnus buildbot[1] still complains there are glitches for
> compiling with old Emacsen (including XEmacsen). Maybe it is
> due to some new cl macros. Here is a compilation log I got in
> my 32-bit Cygwin environment:
>
> In registry-db::registry-collect-prune-candidates:
> registry.el:375:31:Warning: reference to free variable `for'
> registry.el:375:35:Warning: reference to free variable `k'
> registry.el:375:37:Warning: reference to free variable `being'
> registry.el:375:43:Warning: reference to free variable `the'
> registry.el:375:47:Warning: reference to free variable `hash-keys'
> registry.el:375:57:Warning: reference to free variable `of'
> registry.el:376:31:Warning: reference to free variable `using'
> registry.el:376:50:Warning: reference to free variable `v'
> registry.el:377:31:Warning: reference to free variable `when'
> registry.el:378:31:Warning: reference to free variable `collect'
This section of the code was always odd. I couldn't edebug it, either:
it gave me "bad `using' clause." It might be safer just to use a plain
`maphash'.
> In end of data:
> registry.el:387:1:Warning: the following functions are not known to be
> defined: cl-remf, cl-loop, hash-values, cl-subseq
> Wrote /Work/gnus/lisp/registry.elc
These parts of the code weren't changed though -- surely we were getting
the same complaints all along?
Eric
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH] Two issues with the gnus-registry
2014-12-19 0:44 ` Katsumi Yamaoka
2014-12-19 2:08 ` Eric Abrahamsen
@ 2014-12-20 3:09 ` Ted Zlatanov
2014-12-20 11:22 ` Katsumi Yamaoka
1 sibling, 1 reply; 24+ messages in thread
From: Ted Zlatanov @ 2014-12-20 3:09 UTC (permalink / raw)
To: ding
On Fri, 19 Dec 2014 09:44:49 +0900 Katsumi Yamaoka <yamaoka@jpl.org> wrote:
KY> This is what Emacs 23.2 said. Logs that XEmacs 21.4 and 21.5
KY> issued are similar.
Can some XEmacs users propose patches?
I'd like to make 24.1 the oldest supported Gnus release but I get into
trouble every time I suggest that :)
Ted
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH] Two issues with the gnus-registry
2014-12-20 3:09 ` Ted Zlatanov
@ 2014-12-20 11:22 ` Katsumi Yamaoka
2014-12-20 13:53 ` Older Emacsen (was: [PATCH] Two issues with the gnus-registry) Ted Zlatanov
0 siblings, 1 reply; 24+ messages in thread
From: Katsumi Yamaoka @ 2014-12-20 11:22 UTC (permalink / raw)
To: ding
Ted Zlatanov <tzz@lifelogs.com> wrote:
> Can some XEmacs users propose patches?
I did it. :)
^ permalink raw reply [flat|nested] 24+ messages in thread
* Older Emacsen (was: [PATCH] Two issues with the gnus-registry)
2014-12-20 11:22 ` Katsumi Yamaoka
@ 2014-12-20 13:53 ` Ted Zlatanov
0 siblings, 0 replies; 24+ messages in thread
From: Ted Zlatanov @ 2014-12-20 13:53 UTC (permalink / raw)
To: ding
On Sat, 20 Dec 2014 20:22:33 +0900 Katsumi Yamaoka <yamaoka@jpl.org> wrote:
KY> Ted Zlatanov <tzz@lifelogs.com> wrote:
>> Can some XEmacs users propose patches?
KY> I did it. :)
Thanks! I haven't started XEmacs in years so I really appreciate your help.
Ted
^ permalink raw reply [flat|nested] 24+ messages in thread
end of thread, other threads:[~2014-12-20 13:53 UTC | newest]
Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-10-24 19:04 [PATCH] Two issues with the gnus-registry Eric Abrahamsen
2014-10-24 20:56 ` Eric Abrahamsen
2014-10-25 19:59 ` Eric Abrahamsen
2014-10-27 15:03 ` Ted Zlatanov
2014-10-27 19:15 ` Eric Abrahamsen
2014-10-28 18:04 ` Eric Abrahamsen
2014-11-07 23:56 ` Eric Abrahamsen
2014-11-08 0:01 ` Eric Abrahamsen
2014-11-08 8:39 ` Eric Abrahamsen
2014-11-10 13:54 ` Ted Zlatanov
2014-11-11 2:55 ` Eric Abrahamsen
2014-11-13 12:05 ` Eric Abrahamsen
2014-11-16 1:04 ` Dan Christensen
2014-11-16 3:24 ` Eric Abrahamsen
2014-12-18 10:07 ` Ted Zlatanov
2014-12-18 15:00 ` Eric Abrahamsen
2014-12-18 15:09 ` Eric Abrahamsen
2014-12-19 0:44 ` Katsumi Yamaoka
2014-12-19 2:08 ` Eric Abrahamsen
2014-12-20 3:09 ` Ted Zlatanov
2014-12-20 11:22 ` Katsumi Yamaoka
2014-12-20 13:53 ` Older Emacsen (was: [PATCH] Two issues with the gnus-registry) Ted Zlatanov
2014-12-19 1:30 ` [PATCH] Two issues with the gnus-registry Ted Zlatanov
2014-10-28 20:10 ` Ted Zlatanov
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).