Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unix.gettimeofday allocates memory #7446

Closed
vicuna opened this issue Jan 1, 2017 · 4 comments · Fixed by #9561
Closed

Unix.gettimeofday allocates memory #7446

vicuna opened this issue Jan 1, 2017 · 4 comments · Fixed by #9561
Milestone

Comments

@vicuna
Copy link

vicuna commented Jan 1, 2017

Original bug ID: 7446
Reporter: markghayden
Status: acknowledged (set by @gasche on 2017-01-01T19:32:56Z)
Resolution: open
Priority: normal
Severity: tweak
Version: 4.04.0
Target version: 4.07.0+dev/beta2/rc1/rc2
Category: standard library
Monitored by: bikal @ygrek

Bug description

Unix.gettimeofday allocates a floating point value instead of returning an unboxed float similar to Sys.time.

Unix.gettimeofday is not really in the standard library, but is included as part of the Ocaml distribution.

@vicuna
Copy link
Author

vicuna commented Jan 1, 2017

Comment author: @gasche

This issue sounds easier to fix than the other your reported so far. Would you be interested in submitting a patch to fix it, preferably as a Github pull request?

The patch that made Sys.time unboxed, contributed by François Bobot, could serve as useful inspiration:

3c76d06

@vicuna
Copy link
Author

vicuna commented Jan 1, 2017

Comment author: @dra27

Full discussion (with links to previous GPRs) for that commit at #281.

If this is done, then there are other functions (Unix.time, for starters) which should also receive the same treatment (also remember that every change made in unix must be made in win32unix, too - but in this case the CI will catch it, as unix.mli is shared).

However, there is a potentially important difference - none of these functions is presently declared external, and it may be important to consider if we want to change that.

@vicuna
Copy link
Author

vicuna commented Jan 1, 2017

Comment author: @gasche

I don't think that we would want to declare external a function that was not external before (if only because that might break compatibility, although I don't think that people would rely on Unix's signature in practice). I would naively assume that cross-compiling information would be enough for callers to be unboxed as well, but maybe I am wrong?

@vicuna vicuna added the stdlib label Mar 14, 2019
@vicuna vicuna added this to the 4.07.0 milestone Mar 14, 2019
@vicuna vicuna added the bug label Mar 20, 2019
@github-actions
Copy link

github-actions bot commented May 9, 2020

This issue has been open one year with no activity. Consequently, it is being marked with the "stale" label. What this means is that the issue will be automatically closed in 30 days unless more comments are added or the "stale" label is removed. Comments that provide new information on the issue are especially welcome: is it still reproducible? did it appear in other contexts? how critical is it? etc.

@github-actions github-actions bot added the Stale label May 9, 2020
@dra27 dra27 removed the Stale label May 13, 2020
xavierleroy pushed a commit that referenced this issue Jun 2, 2020
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.  Neither is needed according to Single Unix Specification and POSIX.

Fixes: #7446
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants