* [bug#49522] [PATCH] weather: Don't look for exported package replacements twice.
@ 2021-07-11 11:56 Christopher Baines
2021-07-19 17:10 ` Ludovic Courtès
2021-08-01 15:45 ` Christopher Baines
0 siblings, 2 replies; 6+ messages in thread
From: Christopher Baines @ 2021-07-11 11:56 UTC (permalink / raw)
To: 49522
There could be performance issues with member here, but only if there are lots
of replacements.
* guix/scripts/weather.scm (all-packages): Return all exported packages, plus
non exported replacements, rather than including exported replacements twice.
---
guix/scripts/weather.scm | 35 ++++++++++++++++++++++++-----------
1 file changed, 24 insertions(+), 11 deletions(-)
diff --git a/guix/scripts/weather.scm b/guix/scripts/weather.scm
index 06312d65a2..0f70dc8460 100644
--- a/guix/scripts/weather.scm
+++ b/guix/scripts/weather.scm
@@ -53,17 +53,30 @@
#:export (guix-weather))
(define (all-packages)
- "Return the list of public packages we are going to query."
- (fold-packages (lambda (package result)
- (match (package-replacement package)
- ((? package? replacement)
- (cons* replacement package result))
- (#f
- (cons package result))))
- '()
-
- ;; Dismiss deprecated packages but keep hidden packages.
- #:select? (negate package-superseded)))
+ "Return the list of packages we are going to query."
+ (let* ((packages
+ (fold-packages (lambda (package result)
+ (cons package result))
+ '()
+
+ ;; Dismiss deprecated packages but keep hidden packages.
+ #:select? (negate package-superseded)))
+ (non-exported-replacement-packages
+ (fold-packages (lambda (package result)
+ (let ((replacement (package-replacement package)))
+ (if (and replacement
+ ;; Avoid double couting replacements
+ ;; that are themselves exported
+ (not (member replacement packages)))
+ (cons replacement result)
+ result)))
+ '()
+
+ ;; Dismiss deprecated packages but keep hidden packages.
+ #:select? (negate package-superseded))))
+ (append
+ packages
+ non-exported-replacement-packages)))
(define (call-with-progress-reporter reporter proc)
"This is a variant of 'call-with-progress-reporter' that works with monadic
--
2.32.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [bug#49522] [PATCH] weather: Don't look for exported package replacements twice.
2021-07-11 11:56 [bug#49522] [PATCH] weather: Don't look for exported package replacements twice Christopher Baines
@ 2021-07-19 17:10 ` Ludovic Courtès
2021-08-01 15:01 ` Christopher Baines
2021-08-01 15:45 ` Christopher Baines
1 sibling, 1 reply; 6+ messages in thread
From: Ludovic Courtès @ 2021-07-19 17:10 UTC (permalink / raw)
To: Christopher Baines; +Cc: 49522
Hi!
Christopher Baines <mail@cbaines.net> skribis:
> There could be performance issues with member here, but only if there are lots
> of replacements.
>
> * guix/scripts/weather.scm (all-packages): Return all exported packages, plus
> non exported replacements, rather than including exported replacements twice.
[...]
> + "Return the list of packages we are going to query."
> + (let* ((packages
> + (fold-packages (lambda (package result)
> + (cons package result))
> + '()
> +
> + ;; Dismiss deprecated packages but keep hidden packages.
> + #:select? (negate package-superseded)))
> + (non-exported-replacement-packages
> + (fold-packages (lambda (package result)
> + (let ((replacement (package-replacement package)))
> + (if (and replacement
> + ;; Avoid double couting replacements
> + ;; that are themselves exported
> + (not (member replacement packages)))
> + (cons replacement result)
> + result)))
> + '()
> +
> + ;; Dismiss deprecated packages but keep hidden packages.
> + #:select? (negate package-superseded))))
> + (append
> + packages
> + non-exported-replacement-packages)))
Is the goal to delete duplicates?
In that case, how about wrapping the existing expression in
(delete-duplicates … eq?) ?
(Note that (member x lst) is O(N) is the number of packages, so the code
above is quadratic in the number of packages, which should be avoided.
Also, packages cannot be meaningfully compared with ‘equal?’ (which is
expensive in case of different packages), so ‘eq?’ should be preferred.)
Thanks,
Ludo’.
^ permalink raw reply [flat|nested] 6+ messages in thread
* [bug#49522] [PATCH] weather: Don't look for exported package replacements twice.
2021-07-19 17:10 ` Ludovic Courtès
@ 2021-08-01 15:01 ` Christopher Baines
0 siblings, 0 replies; 6+ messages in thread
From: Christopher Baines @ 2021-08-01 15:01 UTC (permalink / raw)
To: Ludovic Courtès; +Cc: 49522
[-- Attachment #1: Type: text/plain, Size: 2694 bytes --]
Ludovic Courtès <ludo@gnu.org> writes:
> Hi!
>
> Christopher Baines <mail@cbaines.net> skribis:
>
>> There could be performance issues with member here, but only if there are lots
>> of replacements.
>>
>> * guix/scripts/weather.scm (all-packages): Return all exported packages, plus
>> non exported replacements, rather than including exported replacements twice.
>
> [...]
>
>> + "Return the list of packages we are going to query."
>> + (let* ((packages
>> + (fold-packages (lambda (package result)
>> + (cons package result))
>> + '()
>> +
>> + ;; Dismiss deprecated packages but keep hidden packages.
>> + #:select? (negate package-superseded)))
>> + (non-exported-replacement-packages
>> + (fold-packages (lambda (package result)
>> + (let ((replacement (package-replacement package)))
>> + (if (and replacement
>> + ;; Avoid double couting replacements
>> + ;; that are themselves exported
>> + (not (member replacement packages)))
>> + (cons replacement result)
>> + result)))
>> + '()
>> +
>> + ;; Dismiss deprecated packages but keep hidden packages.
>> + #:select? (negate package-superseded))))
>> + (append
>> + packages
>> + non-exported-replacement-packages)))
>
> Is the goal to delete duplicates?
Not really, the goal is to have a list with no duplicates.
> In that case, how about wrapping the existing expression in
> (delete-duplicates … eq?) ?
>
> (Note that (member x lst) is O(N) is the number of packages, so the code
> above is quadratic in the number of packages, which should be avoided.
> Also, packages cannot be meaningfully compared with ‘equal?’ (which is
> expensive in case of different packages), so ‘eq?’ should be preferred.)
Replacing member with memq seems sensible.
My understanding of delete duplicates is that it's O(N^2), because it
constructs a new list, and keeps searching it in linear time. While that
sounds better, doing a linear search of packages for each replacement
should be quicker, assuming there's only a few replacements compared to
the number of packages.
I'm fine with either though, I don't actually know which is faster, and
it probably doesn't matter anyway. I'll send a patch with a
delete-duplicates approach.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 987 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* [bug#49522] [PATCH] weather: Don't look for exported package replacements twice.
2021-07-11 11:56 [bug#49522] [PATCH] weather: Don't look for exported package replacements twice Christopher Baines
2021-07-19 17:10 ` Ludovic Courtès
@ 2021-08-01 15:45 ` Christopher Baines
2021-09-01 21:30 ` Ludovic Courtès
1 sibling, 1 reply; 6+ messages in thread
From: Christopher Baines @ 2021-08-01 15:45 UTC (permalink / raw)
To: 49522
* guix/scripts/weather.scm (all-packages): Delete duplicates, so that exported
replacements aren't included twice.
---
guix/scripts/weather.scm | 22 ++++++++++++----------
1 file changed, 12 insertions(+), 10 deletions(-)
diff --git a/guix/scripts/weather.scm b/guix/scripts/weather.scm
index 06312d65a2..60a697d1ac 100644
--- a/guix/scripts/weather.scm
+++ b/guix/scripts/weather.scm
@@ -54,16 +54,18 @@
(define (all-packages)
"Return the list of public packages we are going to query."
- (fold-packages (lambda (package result)
- (match (package-replacement package)
- ((? package? replacement)
- (cons* replacement package result))
- (#f
- (cons package result))))
- '()
-
- ;; Dismiss deprecated packages but keep hidden packages.
- #:select? (negate package-superseded)))
+ (delete-duplicates
+ (fold-packages (lambda (package result)
+ (match (package-replacement package)
+ ((? package? replacement)
+ (cons* replacement package result))
+ (#f
+ (cons package result))))
+ '()
+
+ ;; Dismiss deprecated packages but keep hidden packages.
+ #:select? (negate package-superseded))
+ eq?))
(define (call-with-progress-reporter reporter proc)
"This is a variant of 'call-with-progress-reporter' that works with monadic
--
2.32.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-09-03 9:26 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-07-11 11:56 [bug#49522] [PATCH] weather: Don't look for exported package replacements twice Christopher Baines
2021-07-19 17:10 ` Ludovic Courtès
2021-08-01 15:01 ` Christopher Baines
2021-08-01 15:45 ` Christopher Baines
2021-09-01 21:30 ` Ludovic Courtès
2021-09-03 9:25 ` bug#49522: " Christopher Baines
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/guix.git
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 read-only IMAP folder(s) and NNTP newsgroup(s).