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

fix heap cost calculation rounding error #30673

Merged
merged 5 commits into from
Mar 15, 2023

Conversation

tao-stones
Copy link
Contributor

@tao-stones tao-stones commented Mar 10, 2023

Problem

Should round up heap size to next 32k block before div

Summary of Changes

  • refactor the heap cost into a function
  • add test with refraction heap size (the 3rd case would fail before fix)
  • fix size truncation by round up to next 32K block first
  • add feature gate
  • check result of consume_checked if feature is activated

OPTIONAL: Feature Gate Issue: #30679

@tao-stones tao-stones requested a review from Lichtso March 10, 2023 18:11
@tao-stones
Copy link
Contributor Author

@Lichtso if this is a valid change, it probably needs a feature gate, or perhaps can be rolled into other feature gates if possible?

@Lichtso
Copy link
Contributor

Lichtso commented Mar 10, 2023

Yes, it needs a feature gate, and I can't think of any other related one to merge this change into.

@tao-stones tao-stones force-pushed the fix_heap_cost_calculation branch from 443dbf3 to ce86db9 Compare March 10, 2023 20:43
@tao-stones tao-stones added the feature-gate Pull Request adds or modifies a runtime feature gate label Mar 10, 2023
@tao-stones
Copy link
Contributor Author

Yes, it needs a feature gate, and I can't think of any other related one to merge this change into.

Thanks for quick response. Feature gate added.

@codecov
Copy link

codecov bot commented Mar 10, 2023

Codecov Report

Merging #30673 (cd245fd) into master (04e52f5) will decrease coverage by 0.1%.
The diff coverage is 97.0%.

@@            Coverage Diff            @@
##           master   #30673     +/-   ##
=========================================
- Coverage    81.6%    81.6%   -0.1%     
=========================================
  Files         723      723             
  Lines      201672   201646     -26     
=========================================
- Hits       164766   164744     -22     
+ Misses      36906    36902      -4     

@tao-stones tao-stones force-pushed the fix_heap_cost_calculation branch from 70be670 to 1aa3fe8 Compare March 13, 2023 22:04
@tao-stones tao-stones merged commit a4ad0c7 into solana-labs:master Mar 15, 2023
@tao-stones tao-stones deleted the fix_heap_cost_calculation branch March 15, 2023 17:03
yhchiang-sol pushed a commit to yhchiang-sol/solana that referenced this pull request Mar 16, 2023
* add test for refaction heap size, fix size truncating by div op
* add feature gate
* checked result of consume_checked() if feature is activated
jeffwashington pushed a commit to jeffwashington/solana that referenced this pull request Mar 16, 2023
* add test for refaction heap size, fix size truncating by div op
* add feature gate
* checked result of consume_checked() if feature is activated
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-gate Pull Request adds or modifies a runtime feature gate
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants