-
Notifications
You must be signed in to change notification settings - Fork 335
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
Allow printing gas reports from contracts (debug mode) #746
Comments
I like it. I could work on it after what I'm doing now. |
Would it be sufficient to trigget printing a gas report from the contract? I think this is easy to do. Returning the gas information to the contract is a whole different story. |
Hmmm.... would it fit better in On the other hand, the minimal I would need for performance tests is some external call That said, I think it would also be useful (longer term if this is much harder) to return |
I just looked into the implementation again, and it turns out the imports cannot access E.g. here /// Prints a debug message to console.
/// This does not charge gas, so debug printing should be disabled when used in a blockchain module.
pub fn native_debug<A: Api, S: Storage, Q: Querier>(
env: &Environment<A, S, Q>,
message_ptr: u32,
) -> VmResult<()> {
if env.print_debug {
let message_data = read_region(&env.memory(), message_ptr, MAX_LENGTH_DEBUG)?;
let msg = String::from_utf8_lossy(&message_data);
println!("{}", msg);
}
Ok(())
} However, I would be very very (very very) careful exposing gas information to the contract. The moment we do that, someone starts writing contract code that makes assumtions on how gas works. And then everything blows up when gas metering changes or the contract is used on multiple chains with different pricing for CosmWasm gas or Cosmos SDK gas. |
Okay, updated the title to match the last comments |
Hey y'all. I was searching for this capability and @ethanfrey pointed me here. I can't speak to the implications on the VM side, but from an app developer side this would be AMAZING. If it's possible, a query or some way of poking and keeping the value would be better than just logging, of course. That way we can do things like accumulate the gas costs over certain regions of code, stuff like that. I don't even want to share how many hours or days I have spent going through a process of recompiling and running tests just to look at changes in gas usage, trying to hunt down some elusive costs. Anything I can do to help revive this issue? |
Added to next milestone to revive it from forgotten hood |
This is only possible for integration tests which are run by the VM. Unit tests do not have the inforation available. Given that the entire ecosystem is moving away from integration tests, I wonder if it worth investing time here right now. |
Many people are running full stack tests. There is something called cosm-orc which provides a simple Rust interface to run your contract in a real Juno node and get gas reports. They only make sense on real nodes, not in cosmwasm-vm integration tests. Osmosis is also working on a similar tool for this as well. If we run such a tool with a wasmd/junod/osmosisd binary configured to allow debug output, then providing a place to output "gas used up to this point" could be very useful when trying to optimize gas usage. |
Note: using this requires setting up It can be set by running the node with |
Good. Printing the value to stdout (like |
I think that is a good first step. We should make sure it is normalised to be Cosmos SDK gas so people can compare easily. I would like to get feedback from some other framework (like
Plenty of questions to iterate on. I would work on a good-enough version and hope other code can adapt to it. Or adjust later on. For now, I would propose something as follows. A user can type something like:
The vm code would calculate gas used, convert to sdk gas and output something like: { "gas_used": 123456, "message": "contract.rs:345" } |
Hmm, cosmwasm-vm cannot do that. If this is needed we have to do the whole journey from contract into chain like we do with e.g. |
I don't quite see how More to the point - if these logs only appear on the Go side, that's understandable, but can someone please point me to where I can direct that debug output to a file? ( In a pinch I've added my own file logging to wasmd, but I'm sure I was re-inventing this particular wheel and just missed it in the docs... |
Thanks for the input @dakom these are open questions. I think we agree on the need to do so, now we figure out how to do so in the best way. |
Ah, okay, gotcha. In terms of the contract side:
To be clear, the callback approach would be something like:
Please note that this is a closure, not a function pointer. This is very important. It's fine for the FnOnce to be In terms of the chain logging side: I am not sure of the current capabilities, but I would like to have a separate (and configurable) So for example, there could be a general I'm also a fan of some sort of structured logging, even just basic info/warn/error differentiation and a verbosity flag, but this is less essential (i.e. with the above I'm free to do my own string formatting and logic to supress logs, so it's not a big deal) I'm also going to share some thoughts and experiences which may be helpful to get a sense of where I face friction. I expect there are no surprises here, and some of these thoughts have already been shared elsewhere, but I think it's worthwhile to add here:
Tangential point: 100% accurate gas reports in multi-test, with ability to poke at any time during contract lifecycle. Would be amazing! I assume this is just not possible for a variety of reasons, which is why we're only talking about logging on the chain side. |
Thank you for those real-world experiences and requests on your work flow. Great help to direct this development. Currently deps.api.debug (which we would likely extend with gas info) is handled completely in rust (log to stdout) just a boolean flag on the constructor to optionally enable it (and avoid log pollution on production). Providing things like cosmos sdk gas value, or logging anywhere besides stdout/stderr (like some file set by go flags) would require integration with the Go side. Which, while not impossible, would greatly increase the scope and repos required. If that is required for a usable functionality, we need to consider that.
Ideally... we could possibly provide accurate cost for storage and calling wasm to wasm contracts, but we cannot properly simulate bank (even passing funds between wasm contracts), staking, etc as those have native Go implementations and the cost and implementation may (and does) change between minor releases of the cosmos sdk. Furthermore, there is a more or less fixed cost of say 50-60k per transaction for the ante-handlers, tx size, sig verification, deducting fees, etc etc that we cannot address. Testing against a local node with proper Cosmos SDK version / binary is the only way to get those correct numbers. |
Update: we cannot measure gas costs for contract execution as we run them in Rust (not wasm), so we don't meter instructions. We also have no hooks for gas metering in the existing framework |
Ahh, okay... I see how to use the stdout debug logging now. For anyone landing here and getting their feet wet with this:
@ethanfrey I think what confused me, in case this makes its way into documentation, is that for contract developers we're usually just told to start some local chain instance without any tips for silencing the massive amount of logs, and debug messages also don't appear by default. Combined with the fact that we can just call
Digging a bit deeper, is that here? cosmwasm/packages/vm/src/imports.rs Line 353 in 942a346
If so, can we not have a I would be happy to experiment with this a bit, but I'm not entirely clear on the full path from |
Bummer... In the meantime, I was digging further into the codebase. Is my understanding here at least correct-ish?
|
Yes, pretty good. Just some hints that help better understand the codebase:
I think we should track the |
Thanks for the tips @webmaster128 I am stopping for today... but to pick up tomorrow, is this roughly how to make local changes to the vm so I can hack around and try stuff out? SETUP
ITERATIVE DEV
edit: the above works, I can see custom changes in |
fwiw, I added this into
and called In a contract that does pretty much nothing, the actual result gas used is
And when saving some item to storage, the actual result gas used is
I am really not sure what to make of these numbers or if they correlate to anything meaningful |
Welcome to the core of cosmwasm-vm @dakom you are brave 😄 Few people out of Confio have dug so deep. I can't dig more right now, just want to commend you for your perseverance. I'd be happy for a good solution and glad you can try to customise yourself to test this out. |
Thanks @ethanfrey , that is very kind! |
Btw while I am still not sure on what to make for the raw BreakdownconfigWRITE_COST_FLAT = 2000 on-chain resultexternally_used_gas = 5720 SDK gas = 800_800_000_000 / GAS_MULTIPLIER expectation5840 SDK gas = 2000 + ((112 + 16) * 30) expectation calculationfn storage_binary_len(data: &impl Serialize) -> usize {
// to_vec is ultimately a wrapper around serde_json_wasm::to_vec
let bytes:Vec<u8> = cosmwasm_std::to_vec(data).unwrap();
let bytes_len = bytes.len();
// however, I think it gets written as base64, which adds a bit of extra bloat
((bytes_len as f32 / 3.0).ceil() * 4.0) as usize
}
let foo_size = storage_binary_len(&Foo::new()); // 112
let item_namespace_size = storage_binary_len("item-foo"); // 16
let store_foo_item_cost = WRITE_COST_FLAT + ((foo_size + item_namespace_size) * WRITE_COST_PER_BYTE); //5840 |
Two thoughts to jot down here before I forget... What I said above about passing a closure is probably a bad idea since it could be used for non-determinism. If it's at all possible to pass a callback, it should be a function pointer However, even before kicking the tires on that, I am not sure it's possible to guarantee it on a type level, since it ultimately gets interpreted from a plain integer pointer on the VM side. So even if std guaranteed the signature of debug(), someone could replace that with a different debug() More generally, having the VM interpret a pointer as a function and call it just feels like a security hazard to me. So I'm in favor of just logging the gas alongside the string-message more of a note to self: what bothers me about the difference in my external gas used calculation isn't that it's inexect, it's that the estimation which only accounts for the raw storage bytes is higher.. I'd expect it to be lower if there's a difference. Next step would be to find where the base64 encoding is actually happening, if it's happening, and use that to remove a bit of guesswork |
Those calculations look write. This is running in a (forked) wasmd instance, right?
It does not. Only If we don't multiply with that factor, we get Did you calls |
Can you share the execute function you are checking here? And the re-calculation. Value is not base64 encoded. If you are using an Item, the key length is the un-encoded since of the key. (Maybe with 2 byte length prefix). That would be 8 here. Huh, if I do that, I get expected gas of |
There are charged for the response size and events returned. But I assume this measurement is inside the contract, so that wouldn't be there. If you are forking, you could instrument the Basic GasMeter in cosmos-sdk to report every charge: https://github.com/cosmos/cosmos-sdk/blob/v0.45.9/store/types/gas.go#L90-L101 |
Interesting leads!
Here is the execute function:
And I am calling it now with Here is
Gotta go for now, happy to follow-up next week :) |
I'm removing this from the milestone as it is currently not clear how to solve the issue |
This is very useful. In the same way that the mock instance provides Will do a PR for it. |
Another useful profiling / benchmarking tool: a This can be enabled only with Another option could be to make this method feature-gated or build-gated. So that if you compile cosmwasm-vm for release, by example, it is a no-op / fallbacks to Or better: Just print the timestamps to stdout, do not return them to the contract. Will provide a PR for it. |
The debug handler is now fully customizable (pretty much what Mauro suggested but without the need for a new API). See the current default in libwasmvm: So if you set the debug print flag in wasmvm to true, you get the above printed to STDERR. If you fork libwasmvm, you can use your own handler. Changing cosmwasm-vm is not needed anymore. From the contract you trigger a call to theis handler with Closing for now. If you need more than that, let's refine. |
We can calculate the amount of gas used, both by wasm as well as by external callbacks, upon any call to the vm.
When optimizing contracts, it would be good to expose this number. We don't need (can't really use) rust profiling tools, but we can get deterministic gas costs for every run. With this API as well as Api.debug we could build some tools to get insights into what parts of the code are expensive, using the same tools as normal integration tests.
It's been a while since I profiled and I wonder how much is used by JSON parsing, there have been some large impacts with improved parsing. This would be useful to see if #688 among others is worth pursuing.
The text was updated successfully, but these errors were encountered: