unofficial mirror of guile-user@gnu.org 
 help / color / mirror / Atom feed
From: Tomas Volf <~@wolfsden.cz>
To: Maxime Devos <maximedevos@telenet.be>
Cc: guile-user@gnu.org
Subject: Re: How to globally replace core binding?
Date: Tue, 28 Nov 2023 02:37:15 +0100	[thread overview]
Message-ID: <ZWVES6Sbj9Co_veg@ws> (raw)
In-Reply-To: <90c7afb7-9bfd-30f1-c83e-3458067b69f6@telenet.be>

[-- Attachment #1: Type: text/plain, Size: 2078 bytes --]

Hi,

thanks for the answers.

On 2023-11-28 01:51:46 +0100, Maxime Devos wrote:
> (set! copy-file improved-copy-file)
> 
> This replacement 100% functioning assumes no inlining, nobody capturing the
> old copy-file on the top-level, nobody calling the C function directly ...

In that case this is what I will use.  There should be no fundamental problem if
some case will be missed, since I am doing performance optimization only,
correctness is not affected.

> 
> If the original copy-file is implemented in Scheme, then to avoid inlining
> problems, the module defining copy-file should do
> 
> (set! copy-file copy-file).

It looks like it is defined inside libguile/filesys.c, so that is not an option.
And I would like to avoid patching the Guile itself anyway.

> 
> That way, Guile's compiler/optimizer knows that the binding is mutable and
> should not inlined (well, Guile being Guile, every binding is mutable, but
> now it is mutable from the perspective of the inliner too).

Interesting, is there a way to do the same hint from the C code?  Would there be
a reason?  I assume C code cannot be inlined anyway, so there is no need?

> 
> Depending on whether 'copy-file' is just a stand-in for something else and
> depending on how the better copy-file works/how it is ‘better’, it might be
> better to eventually write a patch to replace copy-file with the improved
> better-file, as then the improved copy-file is more widely available. (As a
> long-term thing; for short-term ‘trying things out’, doing set! is much more
> practical.)

What I want to do is to replace (copy-file oldfile newfile) by modified version
with signature (copy-file oldfile newfile [reflink]) with reflink defaulting to
'auto (see man cp for details).

I expect this to be a somewhat controversial change, so I did not intend to send
a patch, however I can, if you think it has a chance of getting merged.

Tomas

-- 
There are only two hard things in Computer Science:
cache invalidation, naming things and off-by-one errors.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

  reply	other threads:[~2023-11-28  1:37 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-11-26 17:42 How to globally replace core binding? Tomas Volf
2023-11-28  0:51 ` Maxime Devos
2023-11-28  1:37   ` Tomas Volf [this message]
2023-11-28  1:57     ` Maxime Devos

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://www.gnu.org/software/guile/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ZWVES6Sbj9Co_veg@ws \
    --to=~@wolfsden.cz \
    --cc=guile-user@gnu.org \
    --cc=maximedevos@telenet.be \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).