feat: loading spinner for assets table

[vacekj]

re-fixes #1381

[changeset-bot]

⚠️ No Changeset found

Latest commit: 5efc371

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

[Valentine1898]

More manual testing is needed, it seems to be loading my balances many times slower than before

[TalDerei]

More manual testing is needed, it seems to be loading my balances many times slower than before

seemed relatively performant to me during testing?

[Valentine1898]

More manual testing is needed, it seems to be loading my balances many times slower than before

seemed relatively performant to me during testing?

In the penumbra-1 chain, my balances take ~21 sec to load
don't merge this PR

[TalDerei]

can you add a changeset?

[TalDerei]

More manual testing is needed, it seems to be loading my balances many times slower than before

seemed relatively performant to me during testing?

In the penumbra-1 chain, my balances take ~21 sec to load don't merge this PR

More manual testing is needed, it seems to be loading my balances many times slower than before

seemed relatively performant to me during testing?

In the penumbra-1 chain, my balances take ~21 sec to load don't merge this PR

interesting, I didn't test penumbra-1

[Valentine1898]

Screen.Recording.2024-08-05.at.17.39.07.mov

[vacekj]

@Valentine1898 ack, debugging..

[TalDerei]

following up, was the performance regression here fixed? I haven't retested it myself yet

[vacekj]

@VanishMax can you try to repro this? I can't seem to repro locally with the test account on mainnet.

[grod220]

I can't reproduce any slowness, but @JasonMHasperhoven can you test this out on testnet with the testnet whale seedphrase given you have an older macbook?

[TalDerei]

I can't reproduce the slowness either using the whale account on mainnet

Screen.Recording.2024-08-19.at.8.41.48.PM.mov

Screen.Recording.2024-08-19.at.8.56.29.PM.mov

[vacekj]

also can't see why the CI is failing but my local is passing - debugging now.

[vacekj]

CI is fixed now, it was an eslint compat problem.

[JasonMHasperhoven]

Using testnet on my 2019 macbook:
https://github.com/user-attachments/assets/1f09cdaf-067e-42a8-8263-32d044f8bf1a

cc @grod220

[grod220]

Using testnet on my 2019 macbook: https://github.com/user-attachments/assets/1f09cdaf-067e-42a8-8263-32d044f8bf1a

This is a compelling test. Why would streaming balances not be basically instant for a slower machine? The balances steps are this:

• Query notes from database
• Combine notes that are the same asset id + account
• Stream results back to the client
• React renders result (via ZQuery)

This leads me to believe there is a react rendering kind of performance drag.

@JasonMHasperhoven, another helpful test would be to make the same query on main and see how fast that is compared to this. If it's faster, that means it's a react rendering issue. If not, that means the rpc method (or db calls) in the extension is slower for lower performance machines.

[Valentine1898]

Screen.Recording.2024-08-05.at.17.39.07.mov

Additional context
I tested on my macbook m3 pro
The mainnet wallet I tested on has about 3000 notes on it
Loading balances in the main branch is 20 times faster for me

[grod220]

Also @vacekj, just realized we want the spinner to be showing (in a place that doesn't block the content) during the entire stream lifetime. At the moment, it looks like it goes away after the first item is loaded. Can we also use Linewave for the spinner (we use it in other places).

[vacekj]

@grod220 good point, will address this now.

[vacekj]

@grod220 addressed in the latest commit. the correct loading indicator will now show until the stream has finished.

[TalDerei]

@vacekj still unclear if there's a performance regression?

[vacekj]

@TalDerei I can't replicate the perf regression on my machine, and don't see any obvious issues with code. Even with the perf. regression, this feels like better UX than before, so would carefully merge and watch for potential use reports.

Alternatively, I can try debugging with @JasonMHasperhoven on his machine to see what's wrong.

@grod220 would love your call here.