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.

Sign up for GitHub

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

Jump to bottom

Use closure for init constraints #2939

Merged
merged 12 commits into from
Sep 7, 2024
Merged

Use closure for init constraints #2939

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
84a9d8f
use closure for init constraints
CanardMandarin Apr 28, 2024
36a8578
Merge branch 'master' into CanardMandarin/use-closure-for-init-constr…
acheroncrypto Aug 30, 2024
ab2b91f
Use closure for the remaining `init`s
acheroncrypto Aug 30, 2024
8e5ecce
Add `#[inline(never)]` before closures
acheroncrypto Aug 30, 2024
0f1afbf
Update CHANGELOG
acheroncrypto Aug 30, 2024
ae8d46c
Update bench
acheroncrypto Aug 30, 2024
2efef72
Remove `#[inline(never)]`s
acheroncrypto Aug 31, 2024
068dfe0
Update bench
acheroncrypto Aug 31, 2024
9f515d7
Merge branch 'master' into CanardMandarin/master
acheroncrypto Sep 7, 2024
8c2e6db
Re-add `#[inline(never)]`s
acheroncrypto Sep 7, 2024
f243fba
Update bench
acheroncrypto Sep 7, 2024
abe47c7
Update notable bench changes
acheroncrypto Sep 7, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ The minor version will be incremented upon a breaking change and the patch versi
- cli: Fix template code shouldn't escape ([#3210](https://github.com/coral-xyz/anchor/pull/3210)).
- idl: Fix using `address` constraint with non-const expressions ([#3216](https://github.com/coral-xyz/anchor/pull/3216)).
- idl: Fix using full path types with `Program` ([#3228](https://github.com/coral-xyz/anchor/pull/3228)).
- lang: Use closures for `init` constraints to reduce the stack usage of `try_accounts` ([#2939](https://github.com/coral-xyz/anchor/pull/2939)).

### Breaking

Expand Down
8 changes: 5 additions & 3 deletions bench/BINARY_SIZE.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,14 @@ The programs and their tests are located in [/tests/bench](https://github.com/co

Solana version: 1.18.17

| Program | Binary Size | - |
| ------- | ----------- | --------------------- |
| bench | 787,968 | 🟢 **-3,040 (0.38%)** |
| Program | Binary Size | - |
| ------- | ----------- | ------------------------ |
| bench | 1,096,096 | 🔴 **+305,088 (38.57%)** |
Comment on lines -19 to +21
Copy link
Collaborator

Choose a reason for hiding this comment

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

I was planning to merge this today, but the bench results show ~38% increase in binary size with this change. Always using closures for init constraints might not be what we want. Decreasing stack usage is certainly more important than binary size, but the difference is still massive.

Thoughts?

Copy link

Choose a reason for hiding this comment

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

I would kinda expect this might happen due to applying inline(never): Closures benefit a lot from being able to inline and remove any redundant logic inside them.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Thanks! I removed the inline(never)s, but it had very little effect on the binary size (+300k rather than +305k).

What we want is to inline them as long as the try_accounts function doesn't use more than 4096 bytes of stack. Not sure if there is a simple way to make the compiler care about the stack limit when deciding whether to inline or not.

Copy link

Choose a reason for hiding this comment

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

if inline had no major contribution in the 38% increase, what else could have caused such a difference ?

Copy link
Collaborator

Choose a reason for hiding this comment

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

The bench program is a rather unusual program that has 90 inits which skews the results. The binary size difference won't be as bad for regular programs.

I think we can get this in as is, since decreasing stack usage of try_accounts is very valuable currently (at least until we get dynamic stack frames).


### Notable changes

- lang: Use closures for `init` constraints to reduce the stack usage of `try_accounts`. Note that the binary size increase is exaggerated for the bench program because it uses unusual number of `init` constraints. Impact for normal programs will be a lot less severe. ([#2939](https://github.com/coral-xyz/anchor/pull/2939)).

---

## [0.30.1]
Expand Down
178 changes: 89 additions & 89 deletions bench/COMPUTE_UNITS.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,95 +16,95 @@ The programs and their tests are located in [/tests/bench](https://github.com/co

Solana version: 1.18.17

| Instruction | Compute Units | - |
| --------------------------- | ------------- | -------------------- |
| accountInfo1 | 573 | 🟢 **-28 (4.66%)** |
| accountInfo2 | 899 | 🟢 **-24 (2.60%)** |
| accountInfo4 | 1,561 | 🟢 **-22 (1.39%)** |
| accountInfo8 | 2,957 | 🟢 **-18 (0.61%)** |
| accountEmptyInit1 | 4,976 | 🟢 **-58 (1.15%)** |
| accountEmpty1 | 649 | 🟢 **-3 (0.46%)** |
| accountEmptyInit2 | 9,590 | 🟢 **-97 (1.00%)** |
| accountEmpty2 | 1,015 | 🟢 **-1 (0.10%)** |
| accountEmptyInit4 | 18,323 | 🟢 **-178 (0.96%)** |
| accountEmpty4 | 1,740 | 🔴 **+3 (0.17%)** |
| accountEmptyInit8 | 35,827 | 🟢 **-342 (0.95%)** |
| accountEmpty8 | 3,193 | 🔴 **+7 (0.22%)** |
| accountSizedInit1 | 5,070 | 🟢 **-36 (0.71%)** |
| accountSized1 | 690 | 🔴 **+22 (3.29%)** |
| accountSizedInit2 | 9,750 | 🟢 **-78 (0.79%)** |
| accountSized2 | 1,069 | 🔴 **+23 (2.20%)** |
| accountSizedInit4 | 18,677 | 🟢 **-160 (0.85%)** |
| accountSized4 | 1,834 | 🔴 **+27 (1.49%)** |
| accountSizedInit8 | 36,426 | 🟢 **-335 (0.91%)** |
| accountSized8 | 3,357 | 🔴 **+31 (0.93%)** |
| accountUnsizedInit1 | 5,190 | 🟢 **-9 (0.17%)** |
| accountUnsized1 | 747 | 🔴 **+45 (6.41%)** |
| accountUnsizedInit2 | 10,033 | 🟢 **-45 (0.45%)** |
| accountUnsized2 | 1,165 | 🔴 **+49 (4.39%)** |
| accountUnsizedInit4 | 19,134 | 🟢 **-125 (0.65%)** |
| accountUnsized4 | 2,004 | 🔴 **+51 (2.61%)** |
| accountUnsizedInit8 | 37,046 | 🟢 **-285 (0.76%)** |
| accountUnsized8 | 3,679 | 🔴 **+53 (1.46%)** |
| boxedAccountEmptyInit1 | 5,078 | 🔴 **+14 (0.28%)** |
| boxedAccountEmpty1 | 740 | 🔴 **+69 (10.28%)** |
| boxedAccountEmptyInit2 | 9,697 | 🟢 **-24 (0.25%)** |
| boxedAccountEmpty2 | 1,125 | 🔴 **+73 (6.94%)** |
| boxedAccountEmptyInit4 | 18,477 | 🟢 **-105 (0.57%)** |
| boxedAccountEmpty4 | 1,886 | 🔴 **+75 (4.14%)** |
| boxedAccountEmptyInit8 | 36,059 | 🟢 **-270 (0.74%)** |
| boxedAccountEmpty8 | 3,435 | 🔴 **+78 (2.32%)** |
| boxedAccountSizedInit1 | 5,157 | 🔴 **+38 (0.74%)** |
| boxedAccountSized1 | 780 | 🔴 **+94 (13.70%)** |
| boxedAccountSizedInit2 | 9,842 | 🟢 **-3 (0.03%)** |
| boxedAccountSized2 | 1,180 | 🔴 **+95 (8.76%)** |
| boxedAccountSizedInit4 | 18,736 | 🟢 **-89 (0.47%)** |
| boxedAccountSized4 | 1,974 | 🔴 **+100 (5.34%)** |
| boxedAccountSizedInit8 | 36,563 | 🟢 **-261 (0.71%)** |
| boxedAccountSized8 | 3,593 | 🔴 **+103 (2.95%)** |
| boxedAccountUnsizedInit1 | 5,271 | 🔴 **+64 (1.23%)** |
| boxedAccountUnsized1 | 839 | 🔴 **+118 (16.37%)** |
| boxedAccountUnsizedInit2 | 10,040 | 🔴 **+25 (0.25%)** |
| boxedAccountUnsized2 | 1,277 | 🔴 **+120 (10.37%)** |
| boxedAccountUnsizedInit4 | 19,108 | 🟢 **-52 (0.27%)** |
| boxedAccountUnsized4 | 2,142 | 🔴 **+123 (6.09%)** |
| boxedAccountUnsizedInit8 | 37,283 | 🟢 **-213 (0.57%)** |
| boxedAccountUnsized8 | 3,903 | 🔴 **+127 (3.36%)** |
| boxedInterfaceAccountMint1 | 1,502 | 🔴 **+130 (9.48%)** |
| boxedInterfaceAccountMint2 | 2,423 | 🔴 **+130 (5.67%)** |
| boxedInterfaceAccountMint4 | 4,256 | 🔴 **+135 (3.28%)** |
| boxedInterfaceAccountMint8 | 7,950 | 🔴 **+139 (1.78%)** |
| boxedInterfaceAccountToken1 | 2,198 | 🔴 **+142 (6.91%)** |
| boxedInterfaceAccountToken2 | 3,803 | 🔴 **+143 (3.91%)** |
| boxedInterfaceAccountToken4 | 7,004 | 🔴 **+146 (2.13%)** |
| boxedInterfaceAccountToken8 | 13,434 | 🔴 **+150 (1.13%)** |
| interfaceAccountMint1 | 1,626 | 🔴 **+154 (10.46%)** |
| interfaceAccountMint2 | 2,788 | 🔴 **+157 (5.97%)** |
| interfaceAccountMint4 | 5,110 | 🔴 **+159 (3.21%)** |
| interfaceAccountMint8 | 9,749 | 🔴 **+161 (1.68%)** |
| interfaceAccountToken1 | 2,296 | 🔴 **+166 (7.79%)** |
| interfaceAccountToken2 | 4,096 | 🔴 **+168 (4.28%)** |
| interfaceAccountToken4 | 7,692 | 🔴 **+171 (2.27%)** |
| interface1 | 774 | 🔴 **+174 (29.00%)** |
| interface2 | 923 | 🔴 **+178 (23.89%)** |
| interface4 | 1,214 | 🔴 **+181 (17.52%)** |
| interface8 | 1,799 | 🔴 **+183 (11.32%)** |
| program1 | 782 | 🔴 **+186 (31.21%)** |
| program2 | 927 | 🔴 **+190 (25.78%)** |
| program4 | 1,210 | 🔴 **+191 (18.74%)** |
| program8 | 1,779 | 🔴 **+195 (12.31%)** |
| signer1 | 779 | 🔴 **+199 (34.31%)** |
| signer2 | 1,074 | 🔴 **+202 (23.17%)** |
| signer4 | 1,657 | 🔴 **+203 (13.96%)** |
| signer8 | 2,826 | 🔴 **+208 (7.94%)** |
| systemAccount1 | 802 | 🔴 **+210 (35.47%)** |
| systemAccount2 | 1,108 | 🔴 **+214 (23.94%)** |
| systemAccount4 | 1,713 | 🔴 **+216 (14.43%)** |
| systemAccount8 | 2,926 | 🔴 **+219 (8.09%)** |
| uncheckedAccount1 | 785 | 🔴 **+222 (39.43%)** |
| uncheckedAccount2 | 1,061 | 🔴 **+225 (26.91%)** |
| uncheckedAccount4 | 1,604 | 🔴 **+226 (16.40%)** |
| uncheckedAccount8 | 2,699 | 🔴 **+231 (9.36%)** |
| Instruction | Compute Units | - |
| --------------------------- | ------------- | --------------------- |
| accountInfo1 | 573 | 🟢 **-28 (4.66%)** |
| accountInfo2 | 899 | 🟢 **-24 (2.60%)** |
| accountInfo4 | 1,561 | 🟢 **-22 (1.39%)** |
| accountInfo8 | 2,957 | 🟢 **-18 (0.61%)** |
| accountEmptyInit1 | 5,014 | 🟢 **-20 (0.40%)** |
| accountEmpty1 | 649 | 🟢 **-3 (0.46%)** |
| accountEmptyInit2 | 9,155 | 🟢 **-532 (5.49%)** |
| accountEmpty2 | 1,015 | 🟢 **-1 (0.10%)** |
| accountEmptyInit4 | 17,465 | 🟢 **-1,036 (5.60%)** |
| accountEmpty4 | 1,740 | 🔴 **+3 (0.17%)** |
| accountEmptyInit8 | 34,115 | 🟢 **-2,054 (5.68%)** |
| accountEmpty8 | 3,193 | 🔴 **+7 (0.22%)** |
| accountSizedInit1 | 5,107 | 🔴 **+1 (0.02%)** |
| accountSized1 | 690 | 🔴 **+22 (3.29%)** |
| accountSizedInit2 | 9,313 | 🟢 **-515 (5.24%)** |
| accountSized2 | 1,069 | 🔴 **+23 (2.20%)** |
| accountSizedInit4 | 17,812 | 🟢 **-1,025 (5.44%)** |
| accountSized4 | 1,834 | 🔴 **+27 (1.49%)** |
| accountSizedInit8 | 34,706 | 🟢 **-2,055 (5.59%)** |
| accountSized8 | 3,357 | 🔴 **+31 (0.93%)** |
| accountUnsizedInit1 | 5,227 | 🔴 **+28 (0.54%)** |
| accountUnsized1 | 746 | 🔴 **+44 (6.27%)** |
| accountUnsizedInit2 | 9,599 | 🟢 **-479 (4.75%)** |
| accountUnsized2 | 1,163 | 🔴 **+47 (4.21%)** |
| accountUnsizedInit4 | 18,276 | 🟢 **-983 (5.10%)** |
| accountUnsized4 | 2,000 | 🔴 **+47 (2.41%)** |
| accountUnsizedInit8 | 35,328 | 🟢 **-2,003 (5.37%)** |
| accountUnsized8 | 3,671 | 🔴 **+45 (1.24%)** |
| boxedAccountEmptyInit1 | 5,106 | 🔴 **+42 (0.83%)** |
| boxedAccountEmpty1 | 740 | 🔴 **+69 (10.28%)** |
| boxedAccountEmptyInit2 | 9,268 | 🟢 **-453 (4.66%)** |
| boxedAccountEmpty2 | 1,125 | 🔴 **+73 (6.94%)** |
| boxedAccountEmptyInit4 | 17,619 | 🟢 **-963 (5.18%)** |
| boxedAccountEmpty4 | 1,886 | 🔴 **+75 (4.14%)** |
| boxedAccountEmptyInit8 | 34,346 | 🟢 **-1,983 (5.46%)** |
| boxedAccountEmpty8 | 3,435 | 🔴 **+78 (2.32%)** |
| boxedAccountSizedInit1 | 5,189 | 🔴 **+70 (1.37%)** |
| boxedAccountSized1 | 780 | 🔴 **+94 (13.70%)** |
| boxedAccountSizedInit2 | 9,411 | 🟢 **-434 (4.41%)** |
| boxedAccountSized2 | 1,180 | 🔴 **+95 (8.76%)** |
| boxedAccountSizedInit4 | 17,879 | 🟢 **-946 (5.03%)** |
| boxedAccountSized4 | 1,974 | 🔴 **+100 (5.34%)** |
| boxedAccountSizedInit8 | 34,842 | 🟢 **-1,982 (5.38%)** |
| boxedAccountSized8 | 3,593 | 🔴 **+103 (2.95%)** |
| boxedAccountUnsizedInit1 | 5,300 | 🔴 **+93 (1.79%)** |
| boxedAccountUnsized1 | 838 | 🔴 **+117 (16.23%)** |
| boxedAccountUnsizedInit2 | 9,609 | 🟢 **-406 (4.05%)** |
| boxedAccountUnsized2 | 1,275 | 🔴 **+118 (10.20%)** |
| boxedAccountUnsizedInit4 | 18,251 | 🟢 **-909 (4.74%)** |
| boxedAccountUnsized4 | 2,138 | 🔴 **+119 (5.89%)** |
| boxedAccountUnsizedInit8 | 35,562 | 🟢 **-1,934 (5.16%)** |
| boxedAccountUnsized8 | 3,895 | 🔴 **+119 (3.15%)** |
| boxedInterfaceAccountMint1 | 1,502 | 🔴 **+130 (9.48%)** |
| boxedInterfaceAccountMint2 | 2,423 | 🔴 **+130 (5.67%)** |
| boxedInterfaceAccountMint4 | 4,256 | 🔴 **+135 (3.28%)** |
| boxedInterfaceAccountMint8 | 7,950 | 🔴 **+139 (1.78%)** |
| boxedInterfaceAccountToken1 | 2,198 | 🔴 **+142 (6.91%)** |
| boxedInterfaceAccountToken2 | 3,803 | 🔴 **+143 (3.91%)** |
| boxedInterfaceAccountToken4 | 7,004 | 🔴 **+146 (2.13%)** |
| boxedInterfaceAccountToken8 | 13,434 | 🔴 **+150 (1.13%)** |
| interfaceAccountMint1 | 1,626 | 🔴 **+154 (10.46%)** |
| interfaceAccountMint2 | 2,788 | 🔴 **+157 (5.97%)** |
| interfaceAccountMint4 | 5,110 | 🔴 **+159 (3.21%)** |
| interfaceAccountMint8 | 9,749 | 🔴 **+161 (1.68%)** |
| interfaceAccountToken1 | 2,296 | 🔴 **+166 (7.79%)** |
| interfaceAccountToken2 | 4,096 | 🔴 **+168 (4.28%)** |
| interfaceAccountToken4 | 7,692 | 🔴 **+171 (2.27%)** |
| interface1 | 774 | 🔴 **+174 (29.00%)** |
| interface2 | 923 | 🔴 **+178 (23.89%)** |
| interface4 | 1,214 | 🔴 **+181 (17.52%)** |
| interface8 | 1,799 | 🔴 **+183 (11.32%)** |
| program1 | 782 | 🔴 **+186 (31.21%)** |
| program2 | 927 | 🔴 **+190 (25.78%)** |
| program4 | 1,210 | 🔴 **+191 (18.74%)** |
| program8 | 1,779 | 🔴 **+195 (12.31%)** |
| signer1 | 779 | 🔴 **+199 (34.31%)** |
| signer2 | 1,074 | 🔴 **+202 (23.17%)** |
| signer4 | 1,657 | 🔴 **+203 (13.96%)** |
| signer8 | 2,826 | 🔴 **+208 (7.94%)** |
| systemAccount1 | 802 | 🔴 **+210 (35.47%)** |
| systemAccount2 | 1,108 | 🔴 **+214 (23.94%)** |
| systemAccount4 | 1,713 | 🔴 **+216 (14.43%)** |
| systemAccount8 | 2,926 | 🔴 **+219 (8.09%)** |
| uncheckedAccount1 | 785 | 🔴 **+222 (39.43%)** |
| uncheckedAccount2 | 1,061 | 🔴 **+225 (26.91%)** |
| uncheckedAccount4 | 1,604 | 🔴 **+226 (16.40%)** |
| uncheckedAccount8 | 2,699 | 🔴 **+231 (9.36%)** |

### Notable changes

Expand Down
2 changes: 2 additions & 0 deletions bench/STACK_MEMORY.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,8 @@ Solana version: 1.18.17

### Notable changes

- lang: Use closures for `init` constraints to reduce the stack usage of `try_accounts` ([#2939](https://github.com/coral-xyz/anchor/pull/2939)).

---

## [0.30.1]
Expand Down
24 changes: 12 additions & 12 deletions lang/syn/src/codegen/accounts/constraints.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -562,7 +562,7 @@ fn generate_constraint_init_group(
// Define the bump and pda variable.
#find_pda

let #field: #ty_decl = {
let #field: #ty_decl = ({ #[inline(never)] || {
// Checks that all the required accounts for this operation are present.
#optional_checks

Expand Down Expand Up @@ -596,8 +596,8 @@ fn generate_constraint_init_group(
return Err(anchor_lang::error::Error::from(anchor_lang::error::ErrorCode::ConstraintTokenTokenProgram).with_account_name(#name_str).with_pubkeys((*owner_program, #token_program.key())));
}
}
pa
};
Ok(pa)
}})()?;
}
}
InitKind::AssociatedToken {
Expand Down Expand Up @@ -633,7 +633,7 @@ fn generate_constraint_init_group(
// Define the bump and pda variable.
#find_pda

let #field: #ty_decl = {
let #field: #ty_decl = ({ #[inline(never)] || {
// Checks that all the required accounts for this operation are present.
#optional_checks

Expand Down Expand Up @@ -671,8 +671,8 @@ fn generate_constraint_init_group(
return Err(anchor_lang::error::Error::from(anchor_lang::error::ErrorCode::AccountNotAssociatedTokenAccount).with_account_name(#name_str));
}
}
pa
};
Ok(pa)
}})()?;
}
}
InitKind::Mint {
Expand Down Expand Up @@ -888,7 +888,7 @@ fn generate_constraint_init_group(
// Define the bump and pda variable.
#find_pda

let #field: #ty_decl = {
let #field: #ty_decl = ({ #[inline(never)] || {
// Checks that all the required accounts for this operation are present.
#optional_checks

Expand Down Expand Up @@ -980,8 +980,8 @@ fn generate_constraint_init_group(
return Err(anchor_lang::error::Error::from(anchor_lang::error::ErrorCode::ConstraintMintTokenProgram).with_account_name(#name_str).with_pubkeys((*owner_program, #token_program.key())));
}
}
pa
};
Ok(pa)
}})()?;
}
}
InitKind::Program { owner } | InitKind::Interface { owner } => {
Expand Down Expand Up @@ -1033,7 +1033,7 @@ fn generate_constraint_init_group(
// Define the bump variable.
#find_pda

let #field = {
let #field = ({ #[inline(never)] || {
// Checks that all the required accounts for this operation are present.
#optional_checks

Expand Down Expand Up @@ -1078,8 +1078,8 @@ fn generate_constraint_init_group(
}

// Done.
pa
};
Ok(pa)
}})()?;
}
}
}
Expand Down
Loading
Loading