[m-rev.] for review: use the Windows API file copying functions

Julien Fischer jfischer at opturion.com
Sun Jan 7 21:20:59 AEDT 2024


On Sun, 7 Jan 2024, Zoltan Somogyi wrote:

>
> On 2024-01-07 20:36 +11:00 AEDT, "Julien Fischer" <jfischer at opturion.com> wrote:
>> On my Windows machine, this reduces the time required to compile
>> samples/diff using mmc --make from ~30s (with --install-method external)
>> to ~6s (with --install-method internal).
>
> I presume the 6 seconds or so was with CopyFileW.

Yes.

> What was the time with the now renamed do_copy_file?

~11 seconds.

>
>> Use the Windows API file copying functions.
>
> ... on Windows.
>
> Just to prevent people from wondering how this would be done on Linux/MacOS.

Done.

>> +:- func get_internal_copy_method = internal_copy_method.
>> +
>> +:- pragma foreign_proc("C",
>> +    get_internal_copy_method = (Method::out),
>> +    [will_not_call_mercury, thread_safe, promise_pure],
>> +"
>> +#if defined(MR_WIN32) && !defined(MR_CYGWIN)
>> +    Method = MC_ICM_WINDOWS_API;
>> +#else
>> +    Method = MC_ICM_MERCURY_IMPL;
>> +#endif
>> +
>> +").
>> +
>> +get_internal_copy_method = icm_mercury.
>
> Add a comment on the last clause about applicability.

Done.

>> +:- pred do_windows_copy_file(file_name::in, file_name::in, bool::out,
>> +    system_error::out, io::di, io::uo) is det.
>> +
>> +:- pragma foreign_proc("C",
>> +    do_windows_copy_file(Src::in, Dst::in, IsOk::out, SysErr::out,
>> +        _IO0::di, _IO::uo),
>> +    [will_not_call_mercury, promise_pure, thread_safe, tabled_for_io],
>> +"
>> +#if defined(MR_WIN32)
>> +     if (CopyFileW(MR_utf8_to_wide(Src), MR_utf8_to_wide(Dst), FALSE)) {
>> +         IsOk = MR_YES;
>> +         SysErr = 0;
>> +     } else {
>> +         IsOk = MR_NO;
>> +         SysErr = GetLastError();
>> +     }
>> +#else
>> +     MR_fatal_error(""do_windows_copy_file/6 not supported on this system"");
>> +#endif
>> +").
>
> I am far from a Windows native programmer, so you may want to ask
> Peter as well, but that looks OK to me.

I'll deal with any review comments from Peter post-commit; at the moment
--install-method internal is not enabled by default.

Thanks for the review.

Julien.


More information about the reviews mailing list