Skip to content
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

Remove unnecessary gasComputation.Reset call #4535

Merged
merged 2 commits into from
Oct 2, 2022

Conversation

iulianpascalau
Copy link
Contributor

@iulianpascalau iulianpascalau commented Oct 2, 2022

Description of the reasoning behind the pull request

  • gasComputation component started to consume large amounts of time when the calls to SetGasProvided were done on a several hundreds of transactions and up. The time consumed was caused that each transaction hash from the miniblock was added as record in the map that was latter used to append each executed hash, causing exponential time of execution consumed.

Proposed Changes

  • removed unnecessary gasComputation.Reset call from the basePreProcess.handleProcessTransactionInit call

Testing procedure

  • standard system tests
  • half network tests, before and after the partial miniblock execution enable epoch

@codecov-commenter
Copy link

Codecov Report

Base: 73.93% // Head: 73.93% // Increases project coverage by +0.00% 🎉

Coverage data is based on head (3033c1c) compared to base (a7f97d6).
Patch has no changes to coverable lines.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #4535   +/-   ##
=======================================
  Coverage   73.93%   73.93%           
=======================================
  Files         679      679           
  Lines       86969    86968    -1     
=======================================
+ Hits        64298    64301    +3     
+ Misses      17866    17864    -2     
+ Partials     4805     4803    -2     
Impacted Files Coverage Δ
process/block/preprocess/basePreProcess.go 80.95% <ø> (-0.09%) ⬇️
statusHandler/statusMetricsProvider.go 97.92% <0.00%> (+0.82%) ⬆️
storage/txcache/txListBySenderMap.go 100.00% <0.00%> (+2.50%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@iulianpascalau iulianpascalau changed the title Reset unnecessary gasComputation.Reset call Remove unnecessary gasComputation.Reset call Oct 2, 2022
Copy link
Collaborator

@gabi-vuls gabi-vuls left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

System test passed.

@iulianpascalau iulianpascalau merged commit 3b2f905 into master Oct 2, 2022
@iulianpascalau iulianpascalau deleted the remove-unnecessary-reset-calls branch October 2, 2022 18:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants