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

Improve the performance of my-cards page #45

Closed
hantuzun opened this issue Jan 16, 2023 · 13 comments · Fixed by #51
Closed

Improve the performance of my-cards page #45

hantuzun opened this issue Jan 16, 2023 · 13 comments · Fixed by #51
Assignees

Comments

@hantuzun
Copy link
Contributor

When I've 11 cards, https://regen.bingo/my-cards takes a couple of seconds to load. It should load immidiately.

@oytuncoban
Copy link
Contributor

When I've 11 cards, https://regen.bingo/my-cards takes a couple of seconds to load. It should load immidiately.

So, was it a rendering issue or took a while to fetch from the contract?

@hantuzun
Copy link
Contributor Author

It must be about how we fetch the contract; rendering cannot take this long. I'm profiling it now.

@hantuzun
Copy link
Contributor Author

Screen Shot 2023-01-17 at 11 56 32

There are infinite number of fetches for the tokenURI; there is a bug on that.

@oytuncoban
Copy link
Contributor

If we use limit&offset while getting multiple tokens from the contract we are going to need a count function too. And this makes us have to sort the tokens in decreasing order by match count "on the contract". I don't think this is suitable for gas price.

Screen Shot 2023-01-17 at 11 56 32

There are infinite number of fetches for the tokenURI; there is a bug on that.

I know the fix, done in my local. Will be pushed with pagination feature.

@hantuzun
Copy link
Contributor Author

Please make different fixes in different commits and branches.

@hantuzun
Copy link
Contributor Author

The first tokenURI calls are finalized in t1.8s but the first render begins at t8.2s.

Screen Shot 2023-01-17 at 12 00 55

Screen Shot 2023-01-17 at 11 58 04

@oytuncoban
Copy link
Contributor

The first tokenURI calls are finalized in t1.8s but the first render begins at t8.2s.

Screen Shot 2023-01-17 at 12 00 55 Screen Shot 2023-01-17 at 11 58 04

This leads us to decoding the response and creating Card object is taking long. I'm going to optimize it then?

@oytuncoban
Copy link
Contributor

Please make different fixes in different commits and branches.

Done in #47

@hantuzun
Copy link
Contributor Author

Thanks!

@hantuzun hantuzun linked a pull request Jan 17, 2023 that will close this issue
@hantuzun hantuzun reopened this Jan 17, 2023
@hantuzun
Copy link
Contributor Author

hantuzun commented Jan 17, 2023

This issue is not fixed yet. I tried /my-cards again on the deployment of ac8f4f5. It took 9 seconds. Below is the performance display of Brave:

Screen Shot 2023-01-17 at 15 09 12

There are 4 visible requests to the smart contract; and the render happens only after the last one.

@oytuncoban
Copy link
Contributor

oytuncoban commented Jan 17, 2023

This issue is not fixed yet. I tried /my-cards again on the deployment of ac8f4f5. It took 9 seconds. Below is the performance display of Brave:

Screen Shot 2023-01-17 at 15 09 12

There are 4 visible requests to the smart contract; and the render happens only after the last one.

Is that mean fetching cards within a for loop from the contract takes too long when deployed to Göerli? I couldn't reproduce on deployed product since I have no minted cards and minting period has ended, unfortunately.

@hantuzun
Copy link
Contributor Author

Perhaps, needs to be investigated. One way to do would be logging timestamps on relevant requests and seeing them in action with local and Goerli deployments.

@oytuncoban
Copy link
Contributor

oytuncoban commented Jan 19, 2023

Perhaps, needs to be investigated. One way to do would be logging timestamps on relevant requests and seeing them in action with local and Goerli deployments.

Previous approach was;

  • Fetch the tokenId
  • Await tokenURI
  • Await decoding and constructing the Card
  • Add to a list
  • After all the tokens have fetched and Cards are created, set the card's state.

This makes the contract calls last even longer. So, I've decided to update the card's state as a new card have constructed and render it immediately.

I've created another branch and tested a new approach on Göerli. I think it does fine, but this approach is to render the cards immediately after we fetch them so, cards will be loaded one by one. If you agree with me on this approach, we can display a loading spinner until all the cards are fetched at the end of the list @hantuzun.

@oytuncoban oytuncoban linked a pull request Jan 19, 2023 that will close this issue
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.

2 participants