Skip to content

Conversation

benvinegar
Copy link
Contributor

Improves performance of TimeSince component by removing the forceUpdate call we were using to trigger relative time updates.

Now we update relative time via state.relative – React handles the rest.

Makes a noticeable difference on the stream view, where rendering goes from ~25ms on 25 stream items down to ~2ms.

Note this is targeted against the react-0.14 branch.

Copy link
Member

Choose a reason for hiding this comment

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

doesnt this mean 1s doesnt update until its 61s?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, that's true (it was doing that before, too, except every 43 secs).

We should make the timeout according to the duration of the relative time. Follow-up?

Copy link
Member

Choose a reason for hiding this comment

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

yeah i guess thats true, dont know why it was 2600

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I kind of just assumed it was a mistake and 3600 was intended.

@benvinegar
Copy link
Contributor Author

Before:

image

After:

image

(Note these charts don't incorporate bar chart perf improvements from #2202.

@benvinegar
Copy link
Contributor Author

Closing because wrong target. See #2207.

@benvinegar benvinegar closed this Oct 19, 2015
@github-actions github-actions bot locked and limited conversation to collaborators Dec 23, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants