From patchwork Thu Jul 15 08:46:35 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andrew Tropin X-Patchwork-Id: 31429 Return-Path: X-Original-To: patchwork@mira.cbaines.net Delivered-To: patchwork@mira.cbaines.net Received: by mira.cbaines.net (Postfix, from userid 113) id C43AD27BC82; Thu, 15 Jul 2021 09:49:11 +0100 (BST) X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on mira.cbaines.net X-Spam-Level: X-Spam-Status: No, score=-2.8 required=5.0 tests=BAYES_00,DKIM_SIGNED, MAILING_LIST_MULTI,RCVD_IN_MSPIKE_H4,RCVD_IN_MSPIKE_WL,SPF_HELO_PASS, T_DKIM_INVALID,URIBL_BLOCKED autolearn=unavailable autolearn_force=no version=3.4.2 Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) by mira.cbaines.net (Postfix) with ESMTPS id 4E6B927BC6B for ; Thu, 15 Jul 2021 09:49:11 +0100 (BST) Received: from localhost ([::1]:54952 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1m3x3O-0003BD-DJ for patchwork@mira.cbaines.net; Thu, 15 Jul 2021 04:49:10 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:48758) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1m3x3G-0003Ao-DF for guix-patches@gnu.org; Thu, 15 Jul 2021 04:49:02 -0400 Received: from debbugs.gnu.org ([209.51.188.43]:35236) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1m3x3G-0002PG-5u for guix-patches@gnu.org; Thu, 15 Jul 2021 04:49:02 -0400 Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1m3x3G-0002b1-5G for guix-patches@gnu.org; Thu, 15 Jul 2021 04:49:02 -0400 X-Loop: help-debbugs@gnu.org Subject: [bug#49547] [PATCH v2 2/4] home-services: Add home-run-on-change-service-type Resent-From: Andrew Tropin Original-Sender: "Debbugs-submit" Resent-CC: guix-patches@gnu.org Resent-Date: Thu, 15 Jul 2021 08:49:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 49547 X-GNU-PR-Package: guix-patches X-GNU-PR-Keywords: patch To: Maxime Devos , 49547@debbugs.gnu.org Received: via spool by 49547-submit@debbugs.gnu.org id=B49547.16263389019916 (code B ref 49547); Thu, 15 Jul 2021 08:49:02 +0000 Received: (at 49547) by debbugs.gnu.org; 15 Jul 2021 08:48:21 +0000 Received: from localhost ([127.0.0.1]:46776 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1m3x2b-0002Zr-Ed for submit@debbugs.gnu.org; Thu, 15 Jul 2021 04:48:21 -0400 Received: from mail-lf1-f43.google.com ([209.85.167.43]:41897) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1m3x2Z-0002Zf-Ra for 49547@debbugs.gnu.org; Thu, 15 Jul 2021 04:48:20 -0400 Received: by mail-lf1-f43.google.com with SMTP id g8so2715584lfh.8 for <49547@debbugs.gnu.org>; Thu, 15 Jul 2021 01:48:19 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=trop-in.20150623.gappssmtp.com; s=20150623; h=from:to:subject:in-reply-to:references:date:message-id:mime-version; bh=VKp0vfPepaR2VcR93+sa8fAlOCx3b4/7bvXW8fkH+6E=; b=b4U60HzB0vOQXaHXzLIz8CchjPE/RrrU2m5PbxuB3zZ6cDHNlTzWs5Fid5LYwjq+E3 G87bbKd4JFdQAQeDGiBMsfPfCl6iUNcqEg8wANeFM43L8a7uYIyoTIuy64RlXO4ykYYz yrYuCbja3HwKOQKmFNFv9pw+asqBZw+EUVa+VErHhkTcj2LcjQeRUcxI+NtdZFuCUdkm G7VqQyGuEOqeWF2nYe9lTvNBe3PXyJM1tg4zEJ/4QVjk7NfCqQq6Wyk02AyIgSdWuY0q rT+30H0sr7pWawexabEDGFV0BFT6TawyE3VJgo3oC2iSCS+ECw6EWyqmBeVwCehJFBkw 7Ncg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:subject:in-reply-to:references:date :message-id:mime-version; bh=VKp0vfPepaR2VcR93+sa8fAlOCx3b4/7bvXW8fkH+6E=; b=VSbvRE7bfgy9uyxLqN26MYJeseZXHOok6kpZ2bmunkhj3vqGDYODcHWeN4wy0nD9YE jyaef/WsFJfSOkCDknB4IIC1sXYU56yjM5m6lJQHN5WZYfXAjuwnjQBdMQmwxyP6ZKoQ 1nMgf8F0DRiOrFPxIdNTX/sS5Zv/qnNMmlOK/xs78lSQw8E6vgjb18H2dMrQO9QYsb52 GuJuj222izyDamdYwjPhBu7fG+lg9KW9Evwq1y6K5LKhFKkZanmfnhMvH7pQCTtcn85b Z6ekb/ux4XCWhXmAqzVgt1nT+y1DPAkvZ7gKt51JnU8bfY6SYUUIZHBl4/3OkPjihgR1 Zb9g== X-Gm-Message-State: AOAM530xYdK4Qayr0tLNiJyE7oivB5pE0A3utk7cnZBEpM9DDpCtzgwA 0EUCfQScSQ0QrxgYSn/qL42xuQ== X-Google-Smtp-Source: ABdhPJxYcApFtmFfX/MYTYUSZb8GUVecn3p3IxVPR36+hXJkr7tA0d4KLI90+DeMEbaUlkaeduY/RA== X-Received: by 2002:a05:6512:3447:: with SMTP id j7mr2447814lfr.558.1626338893674; Thu, 15 Jul 2021 01:48:13 -0700 (PDT) Received: from localhost ([176.52.100.214]) by smtp.gmail.com with ESMTPSA id x10sm356657lfu.263.2021.07.15.01.48.12 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 15 Jul 2021 01:48:12 -0700 (PDT) From: Andrew Tropin In-Reply-To: References: <875yxem679.fsf@trop.in> Date: Thu, 15 Jul 2021 11:46:35 +0300 Message-ID: <87fswg2cd0.fsf@trop.in> MIME-Version: 1.0 X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-BeenThere: guix-patches@gnu.org List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: guix-patches-bounces+patchwork=mira.cbaines.net@gnu.org Sender: "Guix-patches" X-getmail-retrieved-from-mailbox: Patches Maxime Devos 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: Thank you for suggestions!) From 8dd0c06fb64c8b516418cbdf8c385a6c817e7f26 Mon Sep 17 00:00:00 2001 From: Andrew Tropin 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