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

Use a faster alternative to proc.time() #1078

Closed
3 tasks done
wlandau opened this issue Nov 23, 2019 · 2 comments
Closed
3 tasks done

Use a faster alternative to proc.time() #1078

wlandau opened this issue Nov 23, 2019 · 2 comments

Comments

@wlandau
Copy link
Member

wlandau commented Nov 23, 2019

Prework

  • Read and abide by drake's code of conduct.
  • Search for duplicates among the existing issues, both open and closed.
  • Advanced users: verify that the bottleneck still persists in the current development version (i.e. remotes::install_github("ropensci/drake")) and mention the SHA-1 hash of the Git commit you install.

Description

In this Stack Overflow post, I show that there can be a lot of overhead due to proc.time(). (Appears on my work Macbook, but not on my home Linux machine.) Rui Barradas points out that bench::hires_time() is faster. Seems to be >10x faster on Mac OS, Ubuntu 18.04, and RHEL 7. I should probably benchmark it on Windows too.

It is probably not ideal to depend on all of bench, but we can borrow the C code. Then, build_times() and drake_history() need to be compatible with both the new times and the old proc.time() objects.

@wlandau wlandau self-assigned this Nov 23, 2019
@wlandau
Copy link
Member Author

wlandau commented Nov 25, 2019

From #1078, it looks like this is only going to help on Macs. I wonder why proc.time() is slow on Macs specifically...

@wlandau
Copy link
Member Author

wlandau commented Dec 7, 2019

On reflection, this approach has some problems. The C code in bench is pretty involved and needs to be tested and maintained on all the major platforms and compilers. Since proc.time() only appears to be a bottleneck on Macs, all that maintenance and testing does not seem worth it. Plus, if we use it, we only get elapsed time. Dropping user and system time would be a breaking change in drake.

What I can do is add a new log_build_times argument to make() and drake_config() that lets users disable the build times. There are a ton of little ways to make make() a lot faster, and they probably deserve a chapter in the manual.

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

No branches or pull requests

1 participant