Use closure for init constraints

[CanardMandarin]

Hello,

This PR is somewhat related to issues #2915 and #2920.
It seems that with Anchor 0.29, stack usage in try_accounts increased and more easily causes Stack offset of X exceeded max offset of 4096 by X bytes, please minimize large stack variables error.

Existing projects using Anchor could have trouble migrating, and it looks like this question appears a lot in Discord.
Using closures for init constraints helps decrease the stack usage and helps mitigate the issue.

What do you think?

[vercel]

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

A member of the Team first needs to authorize it.

[acheroncrypto]

Using closures for init constraints helps decrease the stack usage and helps mitigate the issue.

That's definitely interesting because using map_err and creating a closure uses more stack space compared to regular match statements. I'm also not sure if this behavior will always be the same with all Rust versions.

Is it possible to add a test that demonstrates the improvement in stack usage?

[nabeel99]

@acheroncrypto @CanardMandarin , are closures always guranteed to be #[inline(never)] ? since that is what we want to achieve any solution that moves all this to a seperate stack frame. this solution is fine as long as the above gurantee is meant to hold.

[nabeel99]

so i tested it out with the following struct , reduced stack usage from 15k to 4k.

#[derive(Accounts)]
pub struct UpdateFooSecond<'info> {
    #[account(
        mut,
        constraint = &foo.load()?.get_second_authority() == second_authority.key,
    )]
    foo: AccountLoader<'info, Foo>,
    second_authority: Signer<'info>,
}

#[derive(Accounts)]
pub struct CreateBar<'info> {
    #[account(
        init,
        seeds = [authority.key().as_ref(), foo.key().as_ref()],
        bump,
        payer = authority, owner = ID,
        space = Bar::LEN + 8
    )]
    bar: AccountLoader<'info, Bar>,
    #[account(
        init,
        payer = authority, owner = ID,
        space = Bar::LEN + 8
    )]
    bar_3: AccountLoader<'info, Bar>,
    #[account(
        init,
        payer = authority, owner = ID,
        space = Bar::LEN + 8
    )]
    bar_4: AccountLoader<'info, Bar>,
    #[account(
        init,
        payer = authority, owner = ID,
        space = Bar::LEN + 8
    )]
    bar_5: AccountLoader<'info, Bar>,
    #[account(
        init,
        payer = authority, owner = ID,
        space = Bar::LEN + 8
    )]
    bar_6: AccountLoader<'info, Bar>,
    #[account(
        init,
        payer = authority, owner = ID,
        space = Bar::LEN + 8
    )]
    bar_7: AccountLoader<'info, Bar>,
    #[account(
        init,
        payer = authority, owner = ID,
        space = Bar::LEN + 8
    )]
    bar_8: AccountLoader<'info, Bar>,
    #[account(
        init,
        payer = authority, owner = ID,
        space = Bar::LEN + 8
    )]
    bar_9: AccountLoader<'info, Bar>,
    #[account(
        init,
        payer = authority, owner = ID,
        space = Bar::LEN + 8
    )]
    bar_10: AccountLoader<'info, Bar>,
    #[account(
        init,
        payer = authority, owner = ID,
        space = Bar::LEN + 8
    )]
    bar_11: AccountLoader<'info, Bar>,
    #[account(
        init,
        payer = authority, owner = ID,
        space = Bar::LEN + 8
    )]
    bar_12: AccountLoader<'info, Bar>,
    #[account(
        init,
        payer = authority, owner = ID,
        space = Bar::LEN + 8
    )]
    bar_13: AccountLoader<'info, Bar>,
    #[account(
        init,
        payer = authority, owner = ID,
        space = Bar::LEN + 8
    )]
    bar_14: AccountLoader<'info, Bar>,
    #[account(
        init,
        payer = authority, owner = ID,
        space = Bar::LEN + 8
    )]
    bar_15: AccountLoader<'info, Bar>,
    #[account(
        init,
        payer = authority, owner = ID,
        space = Bar::LEN + 8
    )]
    bar_16: AccountLoader<'info, Bar>,
    #[account(
        init,
        payer = authority, owner = ID,
        space = Bar::LEN + 8
    )]
    bar_17: AccountLoader<'info, Bar>,
    #[account(
        init,
        payer = authority, owner = ID,
        space = Bar::LEN + 8
    )]
    bar_18: AccountLoader<'info, Bar>,
    #[account(
        init,
        payer = authority, owner = ID,
        space = Bar::LEN + 8
    )]
    bar_19: AccountLoader<'info, Bar>,
    #[account(
        init,
        payer = authority, owner = ID,
        space = Bar::LEN + 8
    )]
    bar_20: AccountLoader<'info, Bar>,
    #[account(mut)]
    authority: Signer<'info>,
    foo: AccountLoader<'info, Foo>,
    system_program: AccountInfo<'info>,
}**
**

Got good results ,
without closure : Error: Function _ZN95_$LT$zero_copy..CreateBar$u20$as$u20$anchor_lang..Accounts$LT$zero_copy..CreateBarBumps$GT$$GT$12try_accounts17hc3d5369b2e707256E Stack offset of **15208** exceeded max offset of **4096** by **11112** bytes, please minimize large stack variables
with closure : Error: Function _ZN95_$LT$zero_copy..CreateBar$u20$as$u20$anchor_lang..Accounts$LT$zero_copy..CreateBarBumps$GT$$GT$12try_accounts17ha6b9d25c9bc115a3E Stack offset of **4632** exceeded max offset of **4096** by **536** bytes, please minimize large stack variables
For now it seems the closures are acting like #[inline(never)] , but idk if this behavior will stay the same across rust versions, one cant at this point in time use attributes on closures so we cant add a the inline attribute explicitly on the closures , tracking rust rfc : rust-lang/rust#15701

[workingjubilee]

For now it seems the closures are acting like #[inline(never)] , but idk if this behavior will stay the same across rust versions, one cant at this point in time use attributes on closures so we cant add a the inline attribute explicitly on the closures , tracking rust rfc : rust-lang/rust#15701

hm? yeah you can.

[workingjubilee]

see rust-lang/rust#15701 (comment)

[nabeel99]

Hey

For now it seems the closures are acting like #[inline(never)] , but idk if this behavior will stay the same across rust versions, one cant at this point in time use attributes on closures so we cant add a the inline attribute explicitly on the closures , tracking rust rfc : rust-lang/rust#15701

hm? yeah you can.

see rust-lang/rust#15701 (comment)

Seems i misread that pr and thought its still not avbl on stable-wrapping in curly braces compiles fine- thanks for the headsup @workingjubilee .

[cavemanloverboy]

You can always add the attribute

fn main () {
    let demo = {
        #[inline(never)] || println!("hello")
    };
    demo();
}

https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=480eca029ed7e86974043c9b717a528e

[nabeel99]

You can always add the attribute

fn main () {

    let demo = {

        #[inline(never)] || println!("hello")

    };

    demo();

}

https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=480eca029ed7e86974043c9b717a528e

This is exactly what i am doing, wrapped it in a inline closure,the only issue which is preventing me from making a pr is, how do we add bench test for this, is there a way to deterministically measure stack frame size usage, need this since i need to add before and after differences as part of the pr.
See this

[LucasSte]

is there a way to deterministically measure stack frame size usage

You can check the generated assembly code for the contract and analyze the largest offset from R10 in load and store instructions.

[workingjubilee]

This is exactly what i am doing, wrapped it in a inline closure,the only issue which is preventing me from making a pr is, how do we add bench test for this, is there a way to deterministically measure stack frame size usage, need this since i need to add before and after differences as part of the pr.

Not really a good one, no, and Rust does not promise that we do not make the stack size bigger for essentially no reason. ( I mean, we'll probably have a reason, but you might think of it as a silly one, so you would see it as No Reason At All. )

[nabeel-uncx]

This is exactly what i am doing, wrapped it in a inline closure,the only issue which is preventing me from making a pr is, how do we add bench test for this, is there a way to deterministically measure stack frame size usage, need this since i need to add before and after differences as part of the pr.

Not really a good one, no, and Rust does not promise that we do not make the stack size bigger for essentially no reason. ( I mean, we'll probably have a reason, but you might think of it as a silly one, so you would see it as No Reason At All. )

Hey ser @workingjubilee could you elaborate on your point, did not exactly understand, also the approach for inline(never) closure is to force rust to call the providing body of the closure in a seperate function as the ebpf vm provides a fresh stack frame for each function so that is what we are trying to achieve via inline(never).

[workingjubilee]

inline(never) and inline(always) are both actually just hints, and the Rust compiler is free to disregard or overrule them when emitting code. See https://doc.rust-lang.org/reference/attributes/codegen.html

[acheroncrypto]

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?

[workingjubilee]

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.

[acheroncrypto]

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.

[nabeel-uncx]

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

[acheroncrypto]

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).

[acheroncrypto]

Finally able to merge this — thank you for the major improvement!