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

[WIP] Cache bumps in a generated struct instead of a BTreeMap #2065

Closed
wants to merge 14 commits into from

Conversation

billythedummy
Copy link
Contributor

@billythedummy billythedummy commented Jul 16, 2022

Finally got round to hacking out the suggestion I brought up in #1367 (comment).

Current status:

  • Think the changes to lang/ work but I'll need a lot of help refactoring the rest of the examples and tests and cli to work with this change.
  • Made this repo to demo the differences in terms of both code and perf

Given the massive breaking changes this will result in, it's totally understandable if you guys decide that this change isn't worth it

@vercel
Copy link

vercel bot commented Jul 16, 2022

@billythedummy is attempting to deploy a commit to the coral-xyz Team on Vercel.

A member of the Team first needs to authorize it.

Comment on lines +25 to +26
// re-export CtorBumps
pub use crate::ctor::CtorBumps;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

had to do this because IdlCreateAccounts is a type alias for Ctor so the generated bumps struct was CtorBumps instead of IdlCreateAccountsBumps. See https://github.com/coral-xyz/anchor/pull/2065/files#diff-40361330a9a3230c38138958631e0fa34181581c2cedcb74a4f6210067045851R28

@Henry-E
Copy link
Contributor

Henry-E commented Nov 28, 2022

It's definitely an interesting idea. I will try to confer with armani at some stage about the relative benefits / arguments for using a generated struct for bumps instead of a btree map. There must have a good reason to go with a btreemap in the first place but can definitely consider the generated struct approach going forward.

@Aursen
Copy link
Contributor

Aursen commented Jun 9, 2023

Hey @billythedummy, any news about this?

@billythedummy
Copy link
Contributor Author

billythedummy commented Jun 12, 2023

Hey @billythedummy, any news about this?

sry i pretty much forgot about this PR bec i was working on other stuff. The impl is pretty much done here, i just couldnt be bothered to modify the tests if this wasn't going to be merged in anyway. Probably better to close this and restart from latest master at this point in time if this is something people want

@Aursen
Copy link
Contributor

Aursen commented Jun 12, 2023

Okay, I'll take care of it, so thank you very much for all your hard work

@armaniferrante
Copy link
Member

Closing as stale.

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 this pull request may close these issues.

4 participants