Unbox Unix.gettimeofday and Unix.time

[stedolan]

This patch makes Unix.time and Unix.gettimeofday be unboxed and @noalloc, which makes them about 20% faster (as measured by a stupid benchmark that does them many times in a loop).

This removes the fallback and error-handling paths from gettimeofday. I don't think they're needed: gettimeofday was in the 1994 Single Unix Specification and every subsequent POSIX release, and POSIX specifies that it has no errors (and no portable way to detect errors) if the timezone argument is null.

(Resolves #7446)

[dra27]

I think this only works because we install unix.cmx? Is that worth documenting somewhere?

[stedolan]

I think this only works because we install unix.cmx? Is that worth documenting somewhere?

It's the same as any other library, stdlib or otherwise - inlining optimisations only apply if the cmx is around. I don't think there's any need to document this case specially.

[dra27]

This is all looking fine to me, except for the detail on HAS_GETTIMEOFDAY. It's still used in caml_sys_random_seed when gettimeofday if available and time otherwise. I think one of the two following things should happen:

1. caml_sys_random_seed should unconditionally use gettimeofday and the #error you've put in otherlibs/unix/gettimeofday.c can go. configure.ac should fail if gettimeofday is not found (except on Windows).
2. The unix library should be disabled if gettimeofday is not found (again, except on Windows).

For compatibility, in either case we should continue to write HAS_GETTIMEOFDAY to s.h. My preference would be to widen the scope of the PR slightly and do the first one, but I don't like having a #error doing configure's work.

[stedolan]

I think option 1 is wrong. My understanding is that the core system (compiler and bytecode runtime) targets only C99, so can't have a POSIX dependency and should continue to have fallback paths for non-POSIX systems without gettimeofday.

I don't see why we need to disable unix in case 2, though. If you're compiling the unix library on a non-POSIX system, you'll get build errors when e.g. fcntl.h is not found and fcntl() cannot be linked to. There's no configure detection or fallback code here, as the unix library assumes it's being compiled on a POSIX system. What's special about gettimeofday?

[dra27]

You are indeed correct that the option 1 suggestion is broken.

In the correct world, a successful run of configure should always lead to a successful build, because configure should otherwise have failed. In other words, configure is responsible for ensuring that the Posix environment is present and should otherwise disable the Unix library (unless you specify --enable-unix-lib in which case it should error). This was certainly correct for ancient things like vmthreads (which ensured that all the Posix bits it needed were there before building) - however, I accept that that's already not quite right, but in which case the alternative is just to leave the #error out completely (i.e. we get the same linking problem as you'd get if fnctl etc. were missing), and the configure.ac changes can be hardened at a later date.

[stedolan]

OK, I've removed my #error stuff from gettimeofday.c

[dra27]

LGTM, assuming CI passes - changes to configure can (and probably should) go in another PR, given that they're not strictly related or a prerequisite of this change

[xavierleroy]

OK, let's have some good, unboxed time!