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

feat(common-lib): derive dynamic field instead of call to fullnode #246

Conversation

Tzal3x
Copy link
Collaborator

@Tzal3x Tzal3x commented Oct 14, 2024

  • Upgrade @mysten/sui to 1.12.0
    This version includes the deriveDynamicFieldID
    function needed to calculate the DF object ID
    without making a request to a fullnode.
  • Replace client.getDynamicFieldObject with deriveDynamicFieldID.
  • Skip page_fetching benchmarks. The results we get from it are misleading and slow down development whenever there are changes happening in its' dependencies. In the near future we should revisit how to properly bench this. Use multiGetObjects to fetch both the display and the dynamic field object in the same request #250

TODO (following PR)

  • ⚡ Performance improvement: Use multiGetObjects to fetch both the display and the dynamic field object in the same request.

Copy link

vercel bot commented Oct 14, 2024

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

Name Status Preview Comments Updated (UTC)
walrus-sites-sp ✅ Ready (Inspect) Visit Preview 💬 Add feedback Oct 15, 2024 0:43am
walrus-sites-sp-devnet-fallback ✅ Ready (Inspect) Visit Preview 💬 Add feedback Oct 15, 2024 0:43am
walrus-sites-sw ✅ Ready (Inspect) Visit Preview 💬 Add feedback Oct 15, 2024 0:43am

@Tzal3x Tzal3x changed the title feat(common-lib): feat(common-lib): derive dynamic field instead of call to fullnode Oct 14, 2024
@Tzal3x Tzal3x changed the base branch from main to develop October 14, 2024 12:44
Copy link

codspeed-hq bot commented Oct 14, 2024

CodSpeed Performance Report

Merging #246 will not alter performance

Comparing 240-construct-manually-the-object-id-of-the-dynamic-field-without-needing-to-request-it-from-the-rpc (a87490a) with main (bcf47ce)

Summary

✅ 3 untouched benchmarks

🆕 2 new benchmarks
⁉️ 1 dropped benchmarks

⚠️ Please fix the performance issues or acknowledge them on CodSpeed.

Benchmarks breakdown

Benchmark main 240-construct-manually-the-object-id-of-the-dynamic-field-without-needing-to-request-it-from-the-rpc Change
🆕 fetchResource: fetch the flatlander site (with redirects) N/A 4.7 ms N/A
🆕 fetchResource: fetch the landing page site (no redirects) N/A 2.8 ms N/A
⁉️ fetchResource: fetch the flatland walrus site 9.1 ms N/A N/A

This version includes the deriveDynamicFieldID
function needed to calculate the DF object ID
without making a request to a fullnode.
Copy link
Collaborator

@giac-mysten giac-mysten left a comment

Choose a reason for hiding this comment

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

Awesome, thanks @Tzal3x . Generally looks good, only one more important comment.

portal/common/lib/resource.test.ts Show resolved Hide resolved
portal/common/lib/resource.ts Outdated Show resolved Hide resolved
portal/common/lib/resource.ts Show resolved Hide resolved
@giac-mysten giac-mysten merged commit 6bcc382 into develop Oct 15, 2024
16 of 17 checks passed
@giac-mysten giac-mysten deleted the 240-construct-manually-the-object-id-of-the-dynamic-field-without-needing-to-request-it-from-the-rpc branch October 15, 2024 12:46
giac-mysten pushed a commit that referenced this pull request Oct 16, 2024
)

- Upgrade @mysten/sui to 1.12.0
This version includes the `deriveDynamicFieldID`
function needed to calculate the DF object ID
without making a request to a fullnode.
- Replace `client.getDynamicFieldObject` with `deriveDynamicFieldID`.
- Skip `page_fetching` benchmarks. The results we get from it are
misleading and slow down development whenever there are changes
happening in its' dependencies. In the near future we should revisit how
to properly bench this.
#250

TODO (following PR)
- ⚡ Performance improvement: Use `multiGetObjects` to fetch both the
display and the dynamic field object in the same request.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Construct manually the object id of the dynamic field without needing to request it from the RPC
2 participants