-
-
Notifications
You must be signed in to change notification settings - Fork 73
reduce rerenders from run times #842
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
Changes from all commits
d209abd
e2ea590
67a7866
a1fc774
4cc7cca
b25d987
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,26 @@ | ||
| import { useEffect, useState } from "react"; | ||
|
|
||
| import { Total } from "./timings.tsx"; | ||
|
|
||
| export default function LiveTotal({ | ||
| total, | ||
| start, | ||
| }: { | ||
| total: number | undefined; | ||
| start: number; | ||
| }) { | ||
| const sinceStart = () => Date.now() - start; | ||
| const [duration, setDuration] = useState<number>(total ?? sinceStart()); | ||
| useEffect(() => { | ||
| if (total == null) { | ||
| const interval = setInterval(() => { | ||
| setDuration(sinceStart()); | ||
| }, 3001); // to match step polling interval | ||
| return () => clearInterval(interval); | ||
| } else { | ||
| setDuration(total); | ||
| } | ||
| }, [total]); | ||
|
|
||
| return <Total ms={duration} />; | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -401,20 +401,13 @@ function createTimings( | |
| } | ||
| const stage = column.topStage; | ||
|
|
||
| const text = stage?.totalDurationMillis + ""; | ||
| if (!text) { | ||
| // shouldn't happen | ||
| continue; | ||
| } | ||
|
Comment on lines
-405
to
-408
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @timja what was the reason for this having to be accounted for?
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. umm I think was just a safety thing, if it says shouldn't happen then likely wasn't expected in practice |
||
| const key = "l_t_" + node.key; | ||
|
|
||
| labels.push({ | ||
| x: column.centerX, | ||
| y: node.y + 55, | ||
| node, | ||
| stage, | ||
| text, | ||
| key, | ||
| text: "", // we take the duration from the stage itself at render time | ||
| key: `l_t_${node.key}`, | ||
| }); | ||
| } | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I increased this in b25d987
As I was noticing while watching live that numbers were going higher than the final result.
Added + 1 so that this starts just after it and is less likely to go over the final result