-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Remove pass-by-reference return data value from executive #9211
Conversation
Previously we trace call output via |
return match call_result { | ||
MessageCallResult::Success(gas_left, data) => { | ||
let len = cmp::min(output.len(), data.len()); | ||
(&mut output[..len]).copy_from_slice(&data[..len]); |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it took my some time to go through the code, but it lgtm!
@@ -431,7 +438,6 @@ impl<'a, B: 'a + StateBackend> Executive<'a, B> { | |||
Err(evm_err) | |||
} else { | |||
self.state.discard_checkpoint(); | |||
output.write(0, &builtin_out_buffer); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
-1 copy 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, although it's the first time I'm looking at this code. I don't have any suggestion regarding the change in trace output.
rel #6744
Return data is always available from
FinalizationResult
, so using pass-by-reference inExecutive
parameters doesn't give us any performance benefits. This reference also creates some trouble for resumableExecutive
.This PR changes it so that output is read from
FinalizationResult::return_data
. Anunsafe
inevm
crate was also able to be removed due to this change.