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

Initial data #106

Closed
wants to merge 9 commits into from
Closed

Initial data #106

wants to merge 9 commits into from

Conversation

thejessewinton
Copy link
Contributor

@thejessewinton thejessewinton commented Jan 16, 2024

  • Using client components, but starting with initial data from the server, allowing for optimistic UI.
  • Caches all API calls to ensure better performance.
  • Fixes the ReactionButton to work on the feed route as well.

Part of a broader slate of performance work that I'm going to continue doing in the next few weeks.

Copy link

vercel bot commented Jan 16, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
beam ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jan 26, 2024 11:07pm

currentPageNumber === 1
? undefined
: POSTS_PER_PAGE * (currentPageNumber - 1),
})
Copy link
Member

Choose a reason for hiding this comment

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

How long does this query take? If it's slow, we should fix the query before caching it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's coming in about 490ms, so pretty long...

Copy link
Member

Choose a reason for hiding this comment

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

Great info. I checked insights and this query looks like it's max taking 5-10ms. Which is slow for that query & data size, so can likely be optimized. But that's not the source of the 490ms.

Next places to look:

  • Where is the server vs db? Is it network latency?
  • How is the connection established? Is there anything in prisma in between?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Great, I'll keep digging in, get it fixed up.

Copy link
Contributor

Choose a reason for hiding this comment

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

Beam's DB is located in us-east-1:

CleanShot 2024-01-17 at 12 54 07@2x

The Vercel functions are in iad1

CleanShot 2024-01-17 at 12 52 48@2x

Copy link
Contributor Author

@thejessewinton thejessewinton Jan 26, 2024

Choose a reason for hiding this comment

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

@ayrton @mscoutermarsh I've done a fair amount of work here to try and get this faster, and have also improved some of the UI using the initial data.

On the query latency, I'm not seeing anything happening on the Prisma end that would lead to higher latency like this; in some testing, caching the query calls seemed to have done quite a bit to improve load times across the board (176ms). Overall, the Prisma and data implementations are exactly the same as the old one, but in the 2.0 we're getting the data on the server rather than the client, and we're removing loading skeletons, which we had before. Since that would block the TTFB, I think it could be leading to us noticing a slower initial load. Any suggestions for getting this stronger are welcome, I'll try and get them implemented over the weekend.

auto-merge was automatically disabled January 27, 2024 00:17

Pull request was closed

@thejessewinton thejessewinton deleted the initial-data branch January 27, 2024 01:45
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.

3 participants