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

@timed does not report compile/recompile times whereas @time does. #47056

Closed
iago-lito opened this issue Oct 5, 2022 · 2 comments · Fixed by #52889
Closed

@timed does not report compile/recompile times whereas @time does. #47056

iago-lito opened this issue Oct 5, 2022 · 2 comments · Fixed by #52889

Comments

@iago-lito
Copy link

I'm interested in collecting compile/recompile time of the expressions I'm evaluating because they happen to be large and automatically generated.

I can read compilation time from my screen when using @time, but when I attempt to collect this information as data for later use with @timed instead, the compilation/recompilation times seem not to appear within the resulting NamedTuple.

As a workaround, I'm using Base.cumulative_compile_tim* functions because I've found in the sources that this is what @time is using under the hood. But they are not documented and I'm unsure how or whether I'm supposed to do this.

Can the compilation/recompilation times be added to the output of @timed?
As an alternative, will the Base.cumulative_compile_tim* functions be eventually documented and/or exported?

@ericphanson
Copy link
Contributor

it seems to me that @time and @timev should be implemented by printing results from @timed, instead of all 3 having separate implementations. Then @timed would always have all the programmatic info. (Prompted by #52883)

@IanButterworth
Copy link
Member

Yeah I recall originally compile/recompile time was left off as it's a nuanced metric to measure.
But I think consistency in the tooling is good, so makes sense to add to @timed.
Plus I think if people are using internals to do this currently it makes sense to formalize it in the established interfaces.

IanButterworth pushed a commit that referenced this issue Jan 20, 2024
closes #47056

I suspect there was some reason this wasn't done in the first place, but
I figured opening a PR could be a way to discuss that.

~~Needs tests still for the new `@timed` fields.~~

---------

Co-authored-by: inky <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants