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

Added Benchmarking Library #423

Merged
merged 8 commits into from
Nov 20, 2023
Merged

Added Benchmarking Library #423

merged 8 commits into from
Nov 20, 2023

Conversation

wnats
Copy link
Collaborator

@wnats wnats commented Nov 20, 2023

Added a library for benchmarking purposes, as requested by @pschachte.

This is similar to #304, but with @pschachte's suggestion to store the clock counter as a global variable in cbits.c.

This library introduces two new semipure procedures, !benchmark.start and !benchmark.end(?time:float), serving as a pair of probes to measure the CPU clock time elapsed between two points of a program. A utility procedure !benchmark.time_execution(proc:{semipure}(), ?time:float) is also introduced which times proc. The library will throw an error if:

  • !benchmark.start is called twice without a !benchmark.end(?time:float) in between
  • !benchmark.end(?time:float) is called when an un-ended !benchmark.start has not been called
  • !benchmark.time_execution(...) is called when there is a preceding !benchmark.start which has not been ended

@wnats wnats added the enhancement New feature or request label Nov 20, 2023
@wnats wnats requested review from pschachte and jimbxb November 20, 2023 03:05
@wnats wnats self-assigned this Nov 20, 2023
@jimbxb
Copy link
Collaborator

jimbxb commented Nov 20, 2023

This is cool. I like that we can guarantee that the timer is only started and ended once in succession.

One request: can we add a utility of some kind like this?

def {semipure} time(proc:{semipure}(), ?time:float) {
    !start
    !proc
    !end(?time)
}


# Measure the running time of a semipure niladic procedure
# Assumes any start() call has been ended
pub def {semipure} time(proc:{semipure}(), ?time:float) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now that I think about it, we probably want the call_source_location from wherever we call time. I think that means we have to manually call the foreign procs in here too.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point!

}

# End the benchmark clock, yielding the elapsed time in seconds
# Can only be called after a start() has been called
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: can we remove the prepositions on start and end. start doesn't use the preposition :)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No worries, didn't realise that, thank for pointing it out.

@wnats wnats requested a review from jimbxb November 20, 2023 04:25
Copy link
Collaborator

@jimbxb jimbxb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

Copy link
Owner

@pschachte pschachte left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All good except one request.

wybelibs/benchmark.wybe Outdated Show resolved Hide resolved
@wnats wnats requested a review from pschachte November 20, 2023 05:27
@pschachte
Copy link
Owner

Thank you!

@pschachte pschachte merged commit 49bf854 into pschachte:master Nov 20, 2023
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants