Add close unstable #56
Conversation
| @@ -430,17 +436,45 @@ impl AccountInfo { | |||
| // - 8 bytes for the data_len | |||
| // | |||
| // So we can zero out them directly. | |||
| #[cfg(target_os = "solana")] | |||
| sol_memset_(self.data_ptr().sub(48), 0, 48); | |||
There was a problem hiding this comment.
you can use slice::fill and can get rid of the cfg attr
There was a problem hiding this comment.
Using slice::fill takes 58 CUs – same as the current code. The sol_memset_ is more efficient in this case.
There was a problem hiding this comment.
yes, but why?
If the compiler is desugaring to stores, CUs should be lower than the syscall - it's the whole point of applying the optimization. It might mean that the optimization is wrong, or the syscall is mispriced.
There was a problem hiding this comment.
I had the same question. 😊 @LucasSte looked into it and already has a plan to improve it.
There was a problem hiding this comment.
Four stores is the threshold to call memset from compiler-builtins. This is what causing the CU bump. Both the threshold to call the library in LLVM and the threshold to call the syscall in compiler-builtins need to be revised.
| unsafe { | ||
| let zero = 0u64; | ||
| core::arch::asm!( | ||
| "stxdw [{0}-8], {1}", |
There was a problem hiding this comment.
You can try stdw [{0}-8], 0 directly here, without an extra register.
There was a problem hiding this comment.
As per offline conversation, the stdw is not yet available to the compiler – only the vm.
There was a problem hiding this comment.
Yeah, this has been disabled since like forever. You literally can't deploy a program that contains any of the st class insns. If your code never updates the value of r0, you can accomplish the same thing with stxdw[offset], r0 but it's not safe to do so in Rust without some kind of a compiler hint to tell it "please don't reassign r0".
There was a problem hiding this comment.
You literally can't deploy a program that contains any of the st class insns.
Where did you get this information from? Not only does the class pass the bytecode verification, but also is implemented in the jitter and the interpreter.
The fact that the compiler wasn't emitting code for the store immediate class doesn't mean you can't deploy a program with it.
Co-authored-by: Lucas Ste <[email protected]>
Problem
When PR #55 fixed the last index of the owner
Pubkeyonclose_unchecked, it pushed the CUs used by the method to over50.Solution
Reimplement
close_uncheckedusing thesol_memsetsyscall, which consumes14CUs instead. Since this is still over the original7CUs, this PR also re-adds theclose_unstablewhich uses ASM – this variant takes7CUs.The
close_unstablecan be enabled by using the feature"asm".Note:
close_unstablecourtesy of @L0STE and @deanmlittle