Enable displaying mixed resources when using --detailed-resources flag
#3377
Conversation
ddoktorski
requested review from
cptartur and
kkawula
and removed request for
a team
ddoktorski
force-pushed
the
3114-detailed-resources-flag
branch
from
a6e81ae to
7418de8
Compare
| if used_resources.execution_resources != ExecutionResources::default() { | ||
| output.push_str(&vm_resources_output); | ||
| } |
There was a problem hiding this comment.
are we making sure somewhere that resources for the same syscall aren't displayed here multiple times? or maybe quite contrary?
I'm just afraid such situation might be confusing for users (did I just pay twice for a syscall or what happened really?)
There was a problem hiding this comment.
Good point, syscall resources are included only in the tracked resource. This is not ideal because when tracking Sierra gas, gas_consumed includes all syscalls also those from subcalls that tracked cairo steps.
Regarding the double counting of syscall resources, I can add tests for example to assert that the returned VM resources are less than computed from the executed syscalls.
There was a problem hiding this comment.
If some of these costs are included twice, I wonder if this is a good UX to show it then. Maybe just an information that we had to fall back to calculating vm resources in tests and that the values may not be accurate.
There was a problem hiding this comment.
If some of these costs are included twice
No resources are calculated twice. The only issue is that some syscalls, which should be counted as VM resources, are instead included in the Sierra gas consumed. However, they are still only counted once.
Maybe just an information that we had to fall back to calculating vm resources in tests and that the values may not be accurate.
We can add a warning or something like that, but hopefully it will be fixed in #3399
There was a problem hiding this comment.
I think a warning would be a simple thing to add here and it's not sure if #3399 will be addressed quickly.
Related #3114
Introduced changes
sierra-gas, display both Sierra gas and VM resources if VM resources are non zero and--detailed-resourcesflag is usedsierra_gas_consumedvalue