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

Add docs on data-backed ScriptContext #6759

Merged
merged 3 commits into from
Jan 6, 2025
Merged

Add docs on data-backed ScriptContext #6759

merged 3 commits into from
Jan 6, 2025

Conversation

ana-pantilie
Copy link
Contributor

@ana-pantilie ana-pantilie commented Dec 20, 2024

@ana-pantilie ana-pantilie added the No Changelog Required Add this to skip the Changelog Check label Dec 20, 2024
@ana-pantilie ana-pantilie requested review from zliu41 and a team December 20, 2024 16:12
Copy link
Member

@zliu41 zliu41 left a comment

Choose a reason for hiding this comment

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

The documentation should follow the format of having one sentence per line.

As an initial thought, here are some things that should be mentioned - I may have more to add later.

  • When extracting fields from ScriptContext (and in general all Data-encoded types), don't use field accessors, but use record patterns to extract all needed fields at the beginning (and briefly explain why).
  • If your data type includes fields that are lists or maps and you want to wrap it in asData, make sure to use the Data versions of map and list.

@ana-pantilie
Copy link
Contributor Author

don't use field accessors, but use record patterns to extract all needed fields at the beginning (and briefly explain why)

@zliu41 I'm not sure I understand why. As far as I could see, the record patterns are just syntactic sugar over the field accessor functions. I need to look at the way GHC transforms record patterns into Haskell Core again, but I never got the impression the record pattern syntax makes any difference at that level.

I assume you're not referring to the behavior you discovered with matching on pattern synonyms like ScriptContext _ _ _, that the PIR contains some tuple, right?

@zliu41
Copy link
Member

zliu41 commented Dec 23, 2024

Using field accessors leads to repetitive computation. Extracting the third field is head . tail . tail . snd . unsafeDataAsConstr. Later, extracting the fourth field involves the same computation, but one more tail. You must rely on CSE to reduce the repetition, but it is unreliable.

@zliu41
Copy link
Member

zliu41 commented Dec 23, 2024

By the way see this slack thread: https://input-output-rnd.slack.com/archives/G01EE4NQ9U0/p1728897590920289

@zliu41
Copy link
Member

zliu41 commented Dec 23, 2024

Also have a look at AsData.Budget.Spec. Currently patternMatching is most expensive, but if we either turn on CSE, or fix the non-strict variable issue that I told you the other day, then patternMatching will be the most efficient.

Copy link
Member

@zliu41 zliu41 left a comment

Choose a reason for hiding this comment

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

👍

@zliu41 zliu41 merged commit 5460840 into master Jan 6, 2025
97 of 100 checks passed
@zliu41 zliu41 deleted the ana/data-sc-docs branch January 6, 2025 17:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
No Changelog Required Add this to skip the Changelog Check
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants