How to globally replace core binding?

How to globally replace core binding?

[Tomas Volf]

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

Hello,

I would like to replace a core binding.  I know that I can define a module like
this:

    (define-module (foo))
    (define-public (copy-file a b)
      (display "TODO: implement file copy\n"))

And I can later use it

    scheme@(guile-user)> ,use (foo)
    scheme@(guile-user)> (copy-file 'a 'b)
    TODO: implement file copy

But!  I would like to replace the copy-file globally, for all modules, without
the need to explicitly import it.  I guess I want something in the spirit of the
`install-suspendable-ports!', so that I could do something like:

    scheme@(guile-user)> ,use (foo)
    scheme@(guile-user)> (install-better-copy-file!)

I could patch the Guile itself, but I would strongly prefer to avoid that, for
obvious reasons.  So, is it possible to achieve what I want, and if so, how?

Thank you and have a nice day,
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 --]

Re: How to globally replace core binding?

[Maxime Devos]

[-- Attachment #1.1.1: Type: text/plain, Size: 1016 bytes --]

(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 ...

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).

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).

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.)

Best regards,
Maxime Devos.

[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 929 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 236 bytes --]

Re: How to globally replace core binding?

[Tomas Volf]

[-- 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 --]

Re: How to globally replace core binding?

[Maxime Devos]

[-- Attachment #1.1.1: Type: text/plain, Size: 2001 bytes --]

> [...]
> 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.

In that case, this part is N/A.

>> 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?

It would theoretically be possible to implement cross C<->Scheme 
inlining, but given that no such inlining exists currently, there is no 
method for declaring ‘don't inline’ and 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

An extra reflink argument seems fine to me, though I would use a keyword 
argument #:copy-on-write instead -- a bit more flexible towards 
potential future API additions (#:sparse? #true #:user [UID] #:group 
[GID] #:mode [...] ...).

Worst case, someone disagrees that 'auto' should be the default, but 
merely adding the _option_ to copy-on-write would be uncontroversial, I 
think.

Best regards,
MAxime Devos.

[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 929 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 236 bytes --]