-
Notifications
You must be signed in to change notification settings - Fork 137
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
Fix memory metering for loading stored values #2509
Fix memory metering for loading stored values #2509
Conversation
Cadence Benchstat comparisonThis branch with compared with the base branch onflow:feature/stable-cadence commit e857884 Collapsed results for better readability
|
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## feature/stable-cadence #2509 +/- ##
==========================================================
+ Coverage 79.78% 79.79% +0.01%
==========================================================
Files 342 342
Lines 80778 80850 +72
==========================================================
+ Hits 64445 64517 +72
Misses 14024 14024
Partials 2309 2309
Flags with carried forward coverage won't be shown. Click here to find out more.
☔ View full report in Codecov by Sentry. |
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.
Reviewed for code quality; I am not 100% confident on the metering changes themselves.
Added it to the Impl Sync agenda |
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.
The approach makes sense!
I left a couple of comments regarding NewDictionaryMemoryUsages
and NewAtreeMapPreAllocatedElementsMemoryUsage
and a question about DictionaryValue.Clone()
.
b83b7db
to
1bf62fb
Compare
…lue-conversion-metering
Rebased on Stable Cadence |
…lue-conversion-metering
@dsainati1 @SupunS @fxamacker Could you please have another look? |
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.
Nice! 👍 I focused on metering related to atree.
Closes #2507
Description
Remove unnecessary memory metering for loading of stored values.
master
branchFiles changed
in the Github PR explorer