Skip to content

Fix total deltas#187

Merged
IanButterworth merged 1 commit into
KristofferC:masterfrom
IanButterworth:ib/fix_totals
Feb 19, 2025
Merged

Fix total deltas#187
IanButterworth merged 1 commit into
KristofferC:masterfrom
IanButterworth:ib/fix_totals

Conversation

@IanButterworth

@IanButterworth IanButterworth commented Feb 19, 2025

Copy link
Copy Markdown
Collaborator

Pulled out of #184 by @BenCurran98

This PR

julia> begin
       to = TimerOutput()
       @timeit to "1" sleep(1)
       print_timer(to)
       sleep(1)
       print_timer(to)
       end

────────────────────────────────────────────────────────────────────
                           Time                    Allocations
                  ───────────────────────   ────────────────────────
Tot / % measured:      1.00s / 100.0%              384B / 100.0%

Section   ncalls     time    %tot     avg     alloc    %tot      avg
────────────────────────────────────────────────────────────────────
1              1    1.00s  100.0%   1.00s      384B  100.0%     384B
────────────────────────────────────────────────────────────────────
────────────────────────────────────────────────────────────────────
                           Time                    Allocations
                  ───────────────────────   ────────────────────────
Tot / % measured:      1.00s / 100.0%              384B / 100.0%

Section   ncalls     time    %tot     avg     alloc    %tot      avg
────────────────────────────────────────────────────────────────────
1              1    1.00s  100.0%   1.00s      384B  100.0%     384B
────────────────────────────────────────────────────────────────────

Master (note the total time is erroneously 2s in the 2nd print)

julia> begin
       to = TimerOutput()
       @timeit to "1" sleep(1)
       print_timer(to)
       sleep(1)
       print_timer(to)
       end
────────────────────────────────────────────────────────────────────
                           Time                    Allocations
                  ───────────────────────   ────────────────────────
Tot / % measured:      1.07s /  93.5%           4.50MiB /   2.3%

Section   ncalls     time    %tot     avg     alloc    %tot      avg
────────────────────────────────────────────────────────────────────
1              1    1.00s  100.0%   1.00s    105KiB  100.0%   105KiB
────────────────────────────────────────────────────────────────────
────────────────────────────────────────────────────────────────────
                           Time                    Allocations
                  ───────────────────────   ────────────────────────
Tot / % measured:      2.10s /  48.0%           5.43MiB /   1.9%

Section   ncalls     time    %tot     avg     alloc    %tot      avg
────────────────────────────────────────────────────────────────────
1              1    1.00s  100.0%   1.00s    105KiB  100.0%   105KiB
────────────────────────────────────────────────────────────────────

Co-Authored-By: BenCurran98 <73566276+bencurran98@users.noreply.github.com>
@IanButterworth IanButterworth merged commit 82f11d4 into KristofferC:master Feb 19, 2025
@KristofferC

Copy link
Copy Markdown
Owner

To me showing 2s is correct. It's the time and allocations from when you created the timer. We don't start measuring the time from the first section but from when the time is created. I feel the same applies here.

@KristofferC

Copy link
Copy Markdown
Owner

Ref #36

@IanButterworth

Copy link
Copy Markdown
Collaborator Author

Ok. I've blocked registration JuliaRegistries/General#125432

@IanButterworth

Copy link
Copy Markdown
Collaborator Author

I think there's a good argument for the totals to be representative of the contents of the report, particularly if to is being saved and printed later. But #36 does sound like a good way to unambiguate intention.

I just thought this was a clear bug, so I'll revert and leave that to be resolved properly.

IanButterworth added a commit that referenced this pull request Feb 19, 2025
@IanButterworth IanButterworth deleted the ib/fix_totals branch February 19, 2025 17:22
IanButterworth added a commit that referenced this pull request Feb 19, 2025
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 this pull request may close these issues.

2 participants