[bug#49547] [PATCH v2 2/4] home-services: Add home-run-on-change-service-type

[Andrew Tropin]

Maxime Devos <maximedevos@telenet.be> writes:

> Andrew Tropin schreef op ma 05-07-2021 om 18:39 [+0300]:
>> +      (define (equal-regulars? file1 file2)
>> +        "Check if FILE1 and FILE2 are bit for bit identical."
>> +        (let* ((cmp-binary #$(file-append
>> +                              (@ (gnu packages base) diffutils) "/bin/cmp"))
>> +               (status (system* cmp-binary file1 file2)))
>> +          (= status 0)))
>
> Is there any particular reason to shell out to "cmp" instead
> of doing the comparison in Guile?  Starting a process isn't
> the most efficient thing.
>
> Try "time /run/current-system/profile/bin echo", on my system,
> it takes about 2--3 milliseconds for "echo" to finish
> even though it only had to print a newline character.
> Compare with "time echo" (to use the shell built-in "echo"),
> it takes 0.000s seconds on my system.
>
> 3 milliseconds isn't much by itself, but this can accumulate,
> so I would implement the comparison in Guile.
>
> As an optimisation, you could look at the value returned by "lstat".
> If the 'size' is different, then 'equal-regulars?' can return #f
> without reading the file.  If the 'inode' and 'device' are equal,
> then 'equal-regulars?' can return #t I think (at least on conventional
> file systems like btrfs and ext4).

No specific reason.  Yep, spawning a new process can be expensive, but
it's not clear how much time will take the comparison itself and if it
worth it to optimize "startup time". I'm not very fluent with guile
internals and not sure if reimplementation of cmp in guile would improve
or worsen the performance, but it obviously could intoduce some bugs.  I
found Xinglu's idea of the usage of well-tested cmp to be a reasonable
solution here.

Also, this service is expected to be used with small amount of files and
because many of them are symlinks to the store even smaller number of
them will trigger the execution of cmp, so I find the performance
optimization to be preliminary here and propose to address the issue
when and if it appear someday.

However, the ideas about size and inodes are good, easy to implement and
I find them potentially useful to prevent unecessary external process
spawning.  The patch with those improvements are below:

From 8dd0c06fb64c8b516418cbdf8c385a6c817e7f26 Mon Sep 17 00:00:00 2001
From: Andrew Tropin <andrew@trop.in>
Date: Thu, 15 Jul 2021 09:44:30 +0300
Subject: [PATCH] (toberebased) home-services: Prevent unecessary system* call

---
 gnu/home-services.scm | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/gnu/home-services.scm b/gnu/home-services.scm
index 78e5603edf..9afb70f0a7 100644
--- a/gnu/home-services.scm
+++ b/gnu/home-services.scm
@@ -341,8 +341,13 @@ with one gexp, but many times, and all gexps must be 
idempotent.")))
         "Check if FILE1 and FILE2 are bit for bit identical."
         (let* ((cmp-binary #$(file-append
                               (@ (gnu packages base) diffutils) "/bin/cmp"))
-               (status (system* cmp-binary file1 file2)))
-          (= status 0)))
+               (stats1     (lstat file1))
+               (stats2     (lstat file2)))
+          (cond
+           ((= (stat:ino stats1) (stat:ino stats2))         #t)
+           ((not (= (stat:size stats1) (stat:size stats2))) #f)
+
+           (else (= (system* cmp-binary file1 file2) 0)))))
 
       (define (equal-symlinks? symlink1 symlink2)
         "Check if SYMLINK1 and SYMLINK2 are pointing to the same target."
-- 
2.32.0

Thank you for suggestions!)

signature.asc
Description: PGP signature