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

[Clarity] need a .costs-3 contract for Stacks 2.1 and associated benchmarking #3115

Closed
jcnelson opened this issue Apr 27, 2022 · 10 comments
Closed
Assignees
Labels
ship now This is a blocker for the next release (either non-breaking or breaking change) stacks-2.1

Comments

@jcnelson
Copy link
Member

Stacks 2.1 adds new Clarity functions, which will need their own cost functions, which in turn necessitates a .costs-3 contract and associated benchmarks.

@jcnelson
Copy link
Member Author

Making a list:

  • to-consensus-buff: O(n) in the size of the value
  • from-consensus-buff: O(n log n) in the number of atoms in the type signature (caused by tuple key/value pairs needing to be in sorted order).

@saralab
Copy link
Contributor

saralab commented Aug 3, 2022

@jcnelson : @pavitthrap can pick this up once she's back next week, she has prior experience with the costs contract benchmarking and can help with this.
Let me know if I can mark her as the assignee here .

@obycode
Copy link
Contributor

obycode commented Aug 17, 2022

Adding a note to mention that we may want to add a new, separate cost function for parsing with the v2 parser, as it is significantly cheaper than the original parser.

@a3slade
Copy link
Contributor

a3slade commented Oct 14, 2022

Making a list:

  • to-consensus-buff: O(n) in the size of the value
  • from-consensus-buff: O(n log n) in the number of atoms in the type signature (caused by tuple key/value pairs needing to be in sorted order).

I'm not seeing where any sorts are occurring. And perhaps I'm misunderstanding what's happening here, but wouldn't that occur in the serialization?

I don't see runtime_cost called at all from to-consensus-buff. Is it supposed to be or does that n get passed in from somewhere else? In from-consensus-buff it's called with n=the len of the input bytes in the buffer but from Jude's comment that doesn't sound like the n we want (if there is a sort somewhere).

@jcnelson
Copy link
Member Author

jcnelson commented Oct 14, 2022 via email

@a3slade
Copy link
Contributor

a3slade commented Oct 14, 2022

okay, if you're happy then I'm happy.

@a3slade
Copy link
Contributor

a3slade commented Oct 14, 2022

did you see my other question (re: slices and whether they should be O(n) wrt right_position - left_position? Because if you're okay with that being O(1) then I'm ready to lgtm.

@saralab
Copy link
Contributor

saralab commented Oct 24, 2022

To Do:

  • Rebase from next branch and re-run the benchmarking

@jcnelson jcnelson added the ship now This is a blocker for the next release (either non-breaking or breaking change) label Nov 7, 2022
@jcnelson
Copy link
Member Author

jcnelson commented Nov 9, 2022

So in total we need:

  • replace-at?
  • AnalysisFetchContractEntry (thanks @obycode!)
  • as-contract
  • Comparator functions on sequences

@jcnelson
Copy link
Member Author

Merged!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ship now This is a blocker for the next release (either non-breaking or breaking change) stacks-2.1
Projects
None yet
Development

No branches or pull requests

5 participants