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

[protocol_change] Decrease function call base cost #10943

Merged
merged 1 commit into from
Apr 8, 2024

Conversation

aborg-dev
Copy link
Contributor

This PR decreases the function call base execution cost to 1TGas.
It is stacked on #10941.

@aborg-dev aborg-dev requested a review from a team as a code owner April 4, 2024 15:21
@aborg-dev aborg-dev requested review from saketh-are and akhi3030 and removed request for a team and saketh-are April 4, 2024 15:21
@aborg-dev aborg-dev force-pushed the akashin/decrease_function_call_base_cost branch from 3f0d829 to bc979d2 Compare April 4, 2024 15:53
Copy link
Collaborator

@Ekleog-NEAR Ekleog-NEAR left a comment

Choose a reason for hiding this comment

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

I think we need to change more gas costs than just this one in order to avoid a situation later where we’ll want to raise the per-byte cost significantly and not be able to.

To do that, I think we should:

  • Reduce execution costs as decided in the meeting, because while it’s the riskiest change (with the undercharging risk) it’s also the one we expect to get most benefit from on congested shards
  • Reduce send costs to like 100-500Ggas, because while we’re unsure about execution costs, send costs should definitely be much lower than that — and reducing them might help us decongest the shards by the transactions sent by other accounts on the same shard
  • Increase per-byte costs, so that the maximal size of a 4MB contract stays at the same cost. This is because we likely already undercharge for large contracts, and if we don’t increase the per-byte costs at the same time as we decrease the base costs, we won’t be able to increase it later

With these changes, we’ll lower the affine function by keeping the same end and lowering the "0 size" case. The contracts we’re interested in optimizing for are on the lower end of the size spectrum, so we should reap most of the benefits, while avoiding the risk of significant undercharging for large contracts

cc @bowenwang1996 if you have an opinion on this plan?

I likely won’t have the time to do a second round of review today, so I’ll let you land based on Akhi’s approval if the discussion stabilizes, and come back tomorrow otherwise :)

@Ekleog-NEAR
Copy link
Collaborator

This would result in the following numbers:

  • Function call base send: 2_319_861_500_000 -> 200_000_000_000
  • Function call base exec: 2_319_861_500_000 -> 1_000_000_000_000
  • Function call per-byte send: 2_235_934 -> 2_741_348
  • Function call per-byte exec: 2_235_934 -> 2_550_613

This (rounds-down) the total cost for a 4_194_304 bytes contract (our max_contract_size) to the same value as before

@Ekleog-NEAR
Copy link
Collaborator

Erratum: I used the wrong costs, as pointed out by Bowen on Zulip:

The corrected estimations are:

  • Function call base send: 2_319_861_500_000 -> 200_000_000_000 (estimator-provided value: ~10_000_000)
  • Function call base exec: 2_319_861_500_000 -> 1_000_000_000_000 (estimator-provided value: 108_726_617_500)
  • Contract loading per-byte: 216_750 -> 1_036_843 (estimator-provided value: 5_500_000)

@aborg-dev aborg-dev force-pushed the akashin/decrease_function_call_base_cost branch from bc979d2 to c79d18b Compare April 5, 2024 11:18
@aborg-dev aborg-dev force-pushed the akashin/decrease_function_call_base_cost branch 2 times, most recently from a9b19c6 to d25dc8c Compare April 5, 2024 12:14
@aborg-dev
Copy link
Contributor Author

I've updated the costs to suggested values - this PR should be ready for testing on mocknet.

Copy link
Collaborator

@Ekleog-NEAR Ekleog-NEAR left a comment

Choose a reason for hiding this comment

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

LGTM! :) just assuming tests pass fine

core/parameters/src/config_store.rs Show resolved Hide resolved
Copy link
Collaborator

@bowenwang1996 bowenwang1996 left a comment

Choose a reason for hiding this comment

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

Copy paste what I wrote on zulip: I thought the discussion here refers to a later change down the line. The issue is that if we do this change as is, then we don't gain much in terms of improving receipt throughput because when a function call receipt is executed, we charge both exec cost and wasm_contract_loading_bytes. I suggest we go with changing only exec cost now and do other changes after we have a more comprehensive understanding of the performance characteristics of runtime

@Ekleog-NEAR
Copy link
Collaborator

Ekleog-NEAR commented Apr 5, 2024

Copying here the results of the discussion from zulip: it’s probably best to go with option 3 described there, aka:

  • Function call base send: 2_319_861_500_000 -> 200_000_000_000 (estimator-provided value: ~10_000_000)
  • Function call base exec: 2_319_861_500_000 -> 780_000_000_000 (estimator-provided value: 108_726_617_500)
  • Contract loading per-byte: 216_750 -> 1_089_295 (estimator-provided value: 5_500_000)

This gets 300kB contracts down to 1.1Tgas exec cost (the same value as just lowering function call base exec cost to 1Tgas without any other costs changes), while still keeping 4MB contracts at the same cost as before

Base automatically changed from akashin/bounded_refunds to master April 5, 2024 16:32
@VanBarbascu VanBarbascu force-pushed the akashin/decrease_function_call_base_cost branch from 7aab595 to 1265c57 Compare April 5, 2024 17:33
@aborg-dev aborg-dev force-pushed the akashin/decrease_function_call_base_cost branch 6 times, most recently from aa2d8b5 to 146d760 Compare April 6, 2024 05:14
@aborg-dev aborg-dev force-pushed the akashin/decrease_function_call_base_cost branch 5 times, most recently from f86f49c to 66dffdf Compare April 8, 2024 11:46
@aborg-dev aborg-dev force-pushed the akashin/decrease_function_call_base_cost branch 2 times, most recently from ddab7c2 to 3debb22 Compare April 8, 2024 12:01
@aborg-dev aborg-dev enabled auto-merge April 8, 2024 12:02
@aborg-dev aborg-dev force-pushed the akashin/decrease_function_call_base_cost branch from 3debb22 to 2d92347 Compare April 8, 2024 12:24
@aborg-dev aborg-dev force-pushed the akashin/decrease_function_call_base_cost branch from 2d92347 to 6b6d8eb Compare April 8, 2024 13:08
@aborg-dev aborg-dev added this pull request to the merge queue Apr 8, 2024
Copy link

codecov bot commented Apr 8, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 71.37%. Comparing base (22427ed) to head (6b6d8eb).

Additional details and impacted files
@@            Coverage Diff             @@
##           master   #10943      +/-   ##
==========================================
+ Coverage   66.97%   71.37%   +4.39%     
==========================================
  Files         748      760      +12     
  Lines      151539   152771    +1232     
  Branches   151539   152771    +1232     
==========================================
+ Hits       101498   109041    +7543     
+ Misses      45726    39206    -6520     
- Partials     4315     4524     +209     
Flag Coverage Δ
backward-compatibility 0.24% <0.00%> (?)
db-migration 0.24% <0.00%> (?)
genesis-check 1.43% <0.00%> (?)
integration-tests 37.04% <0.00%> (?)
linux 69.86% <100.00%> (+3.53%) ⬆️
linux-nightly 70.85% <100.00%> (+4.36%) ⬆️
macos 54.40% <100.00%> (?)
pytests 1.65% <0.00%> (?)
sanity-checks 1.44% <0.00%> (?)
unittests 67.02% <100.00%> (+0.04%) ⬆️
upgradability 0.29% <0.00%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Merged via the queue into master with commit b2c82f9 Apr 8, 2024
29 checks passed
@aborg-dev aborg-dev deleted the akashin/decrease_function_call_base_cost branch April 8, 2024 13:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants