Skip to content

Commit

Permalink
wip - adding examples
Browse files Browse the repository at this point in the history
  • Loading branch information
tao-stones committed Aug 27, 2024
1 parent c66930e commit 6cb5654
Showing 1 changed file with 13 additions and 5 deletions.
18 changes: 13 additions & 5 deletions proposals/simd-0170-builtin-instruction-cost-and-budget.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,40 +14,48 @@ extends:
---

## Summary
Builtin instructions in the SVM consume their static DEFAULT_COMPUTE_UNITS from the compute budget during execution. These DEFAULT_COMPUTE_UNITS are also counted against block limits during the banking stage. However, historically, builtin instructions have been allocated DEFAULT_INSTRUCTION_COMPUTE_UNIT_LIMIT units of compute budget. This discrepancy between the allowed consumption and the actual usage tracked for block limits creates several issues, which need to be addressed.

Builtin instructions always consume statically defined amount of CUs, they shall allocate exact same amount of compute budget, and shall count same amount towards to block limit at banking stage.

Check failure on line 18 in proposals/simd-0170-builtin-instruction-cost-and-budget.md

View workflow job for this annotation

GitHub Actions / Markdown Linter

Line length [Expected: 80; Actual: 195]

proposals/simd-0170-builtin-instruction-cost-and-budget.md:18:81 MD013/line-length Line length [Expected: 80; Actual: 195]

## Motivation

Builtin instructions in the SVM consume their static DEFAULT_COMPUTE_UNITS from the compute budget during execution. These DEFAULT_COMPUTE_UNITS are also counted against block limits during the banking stage. However, historically, builtin instructions have been allocated DEFAULT_INSTRUCTION_COMPUTE_UNIT_LIMIT units of compute budget. This discrepancy between the allowed consumption and the actual usage tracked for block limits creates several issues, which need to be addressed.

Check failure on line 22 in proposals/simd-0170-builtin-instruction-cost-and-budget.md

View workflow job for this annotation

GitHub Actions / Markdown Linter

Line length [Expected: 80; Actual: 483]

proposals/simd-0170-builtin-instruction-cost-and-budget.md:22:81 MD013/line-length Line length [Expected: 80; Actual: 483]

Allocating DEFAULT_INSTRUCTION_COMPUTE_UNIT_LIMIT instead of the actual DEFAULT_COMPUTE_UNITS for builtin instructions distorts the tracking of transaction compute budgets. This can result in more expensive instructions being executed when they should fail due to exceeding the budget. As a consequence, the cost tracker may need to account for additional compute units towards block limits after transaction execution, potentially producing blocks that exceed those limits.

Check failure on line 24 in proposals/simd-0170-builtin-instruction-cost-and-budget.md

View workflow job for this annotation

GitHub Actions / Markdown Linter

Line length [Expected: 80; Actual: 474]

proposals/simd-0170-builtin-instruction-cost-and-budget.md:24:81 MD013/line-length Line length [Expected: 80; Actual: 474]

Moreover, maintaining consistency in transaction costs between the banking stage and SVM would greatly simplify the code logic and make reasoning more straightforward.

Check failure on line 26 in proposals/simd-0170-builtin-instruction-cost-and-budget.md

View workflow job for this annotation

GitHub Actions / Markdown Linter

Line length [Expected: 80; Actual: 167]

proposals/simd-0170-builtin-instruction-cost-and-budget.md:26:81 MD013/line-length Line length [Expected: 80; Actual: 167]

## Alternatives Considered

One possible alternative approach would be to maintain the current allocation of compute budget for builtin instructions but add logic to the cost tracker to account for the discrepancy during tracking. However, this would add complexity and could introduce additional corner cases, potentially leading to more issues.
- One possible alternative approach would be to maintain the current allocation of compute budget for builtin instructions but add logic to the cost tracker to account for the discrepancy during tracking. However, this would add complexity and could introduce additional corner cases, potentially leading to more issues.

Check failure on line 30 in proposals/simd-0170-builtin-instruction-cost-and-budget.md

View workflow job for this annotation

GitHub Actions / Markdown Linter

Line length [Expected: 80; Actual: 320]

proposals/simd-0170-builtin-instruction-cost-and-budget.md:30:81 MD013/line-length Line length [Expected: 80; Actual: 320]

- Not to treat builtin instructions differently than other instructions, in anothe rwords, to allocate DEFAULT_INSTRUCTION_COMPUTE_UNIT_LIMIT units for builtin instructions as well. The concern with this approach is DEFAULT_INSTRUCTION_COMPUTE_UNIT_LIMIT is rather a very conservative estimation for instruction, currently at 200_000 CU per instruction, estimate all builtin instructions, including votes and transfers, will make banking stage to significantly over reserve block space when producing blocks, potentially lead to under-packed blocks. Further, argument can be made that if it is known that builtin instructions are going to consume this fixed amount CUs, why estimate them with some generic DEFAULT_INSTRUCTION_COMPUTE_UNIT_LIMIT?

Check failure on line 32 in proposals/simd-0170-builtin-instruction-cost-and-budget.md

View workflow job for this annotation

GitHub Actions / Markdown Linter

Line length [Expected: 80; Actual: 745]

proposals/simd-0170-builtin-instruction-cost-and-budget.md:32:81 MD013/line-length Line length [Expected: 80; Actual: 745]

## New Terminology

None

## Detailed Design

1. compute budget allocates actual DEFAULT_COMPUTE_UNITS for builtin instructions, including for compute-budget-instructions.
Propose 3 changes to ensure that: if it is predetermined that a builtin instruction will consume n CUs, then we shall consistently allocate n CUs as budget, and count nCUs towards to block limits. This rule should apply to all builin instructions, including compute-budget instructions.

Check failure on line 40 in proposals/simd-0170-builtin-instruction-cost-and-budget.md

View workflow job for this annotation

GitHub Actions / Markdown Linter

Line length [Expected: 80; Actual: 287]

proposals/simd-0170-builtin-instruction-cost-and-budget.md:40:81 MD013/line-length Line length [Expected: 80; Actual: 287]

1. When compute-unit-limit is not explicitly requested, compute budget always allocates statically defined DEFAULT_COMPUTE_UNITS for builtin instructions, including for compute-budget-instructions.

Check failure on line 42 in proposals/simd-0170-builtin-instruction-cost-and-budget.md

View workflow job for this annotation

GitHub Actions / Markdown Linter

Line length [Expected: 80; Actual: 198]

proposals/simd-0170-builtin-instruction-cost-and-budget.md:42:81 MD013/line-length Line length [Expected: 80; Actual: 198]

Currently if not explicitly requested, each instruction is allocated DEFAULT_INSTRUCTION_COMPUTE_UNIT_LIMIT, capped by transaction's MAX_COMPUTE_UNIT_LIMIT; except for compute-budget instructions - no units are allocated for them, historically. There are few scenarios this could lead to over pack blocks.
Currently when set-compute-unit-limit is not used, all instruction is allocated DEFAULT_INSTRUCTION_COMPUTE_UNIT_LIMIT, capped by transaction's MAX_COMPUTE_UNIT_LIMIT; except for compute-budget instructions - no units are allocated for them, historically. There are few scenarios this could lead to over pack blocks.

Check failure on line 44 in proposals/simd-0170-builtin-instruction-cost-and-budget.md

View workflow job for this annotation

GitHub Actions / Markdown Linter

Line length [Expected: 80; Actual: 317]

proposals/simd-0170-builtin-instruction-cost-and-budget.md:44:81 MD013/line-length Line length [Expected: 80; Actual: 317]

[Example 1](https://github.com/anza-xyz/agave/pull/2746/files#diff-c6c8658338536afbf59d65e9f66b71460e7403119ca76e51dc9125e1719f4f52R13403-R13429):
A transaction consist of a `Transfer` and a expensive non-builtin instruction; by expensive, let's say its actual execution take more than DEFAULT_INSTRUCTION_COMPUTE_UNIT_LIMIT. Because it over budgeted for `Transfer`, allows expensive ix to execute to completion, force adjust up

Check failure on line 47 in proposals/simd-0170-builtin-instruction-cost-and-budget.md

View workflow job for this annotation

GitHub Actions / Markdown Linter

Line length [Expected: 80; Actual: 281]

proposals/simd-0170-builtin-instruction-cost-and-budget.md:47:81 MD013/line-length Line length [Expected: 80; Actual: 281]

[Example 2](https://github.com/anza-xyz/agave/pull/2746/files#diff-c6c8658338536afbf59d65e9f66b71460e7403119ca76e51dc9125e1719f4f52R13431-R13455):
A builtin instruction that might CPIs into other instructions, it'd fail without explicitly request more CU. But it is not atm.

2. cost model honors set_compute_unit_limits, even there is no user space instructions
2. When compute-unit-limit is explicitly requested, always use it reserve block space during block production, even there is no user space instructions

[Example 3](https://github.com/anza-xyz/agave/pull/2746/files#diff-c6c8658338536afbf59d65e9f66b71460e7403119ca76e51dc9125e1719f4f52R13457-R13484)
cost model ignores explicitly requested CU due to all builtin instructions, it ended up to have adjust up, instead of normal adjust down.

Note: users are always encouraged to explicitly requested reasonable amount of compute-unit-limits. Requesting more than what's needed will not only increase the prioritization fee user pays, but also lower priority.

3. fail transaction before send sending for execution if set_compute_unit_limit sets an invalid value.

[Example 4](https://github.com/anza-xyz/agave/pull/2746/files#diff-c6c8658338536afbf59d65e9f66b71460e7403119ca76e51dc9125e1719f4f52R13344-R13373):
Expand Down

0 comments on commit 6cb5654

Please sign in to comment.