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

Feature: Explicitly limit TransactionContext::instruction_trace_capacity #27938

Merged

Conversation

Lichtso
Copy link
Contributor

@Lichtso Lichtso commented Sep 20, 2022

Problem

At the moment the total number of instructions (including CPI) in a transaction is only implicitly limited by the compute budget. The reason for the limit is so that we can calculate an upper bound on the memory that the program-runtime will require for execution, before hand during TX batch loading. This enables us to remove multi-threaded memory management which is the next constraint on parallelism after direct mapping is enabled.

Summary of Changes

  • Renames instruction_context_capacity => instruction_stack_capacity.
  • Replaces number_of_instructions_at_transaction_level by instruction_trace_capacity.
  • Adds InstructionError::MaxInstructionTraceLengthExceeded.
  • Adds ComputeBudget::max_instruction_trace_length which is set to 64.
  • Adds test_max_instruction_trace_length().
  • Adjusts TransactionContext::new() parameter.
  • Adds feature gate limit_max_instruction_trace_length.

Feature Gate Issue: #27939

Number of transactions of a given instruction_trace_length recorded on MNB

  1. 1421
  2. 477252377 (90 percentile)
  3. 13535061
  4. 2669600
  5. 1620364
  6. 2613156
  7. 7535162
  8. 2125437
  9. 11199559
  10. 199152
  11. 611113
  12. 448506 (99 percentile)
  13. 194655
  14. 1018815
  15. 711712
  16. 161004
  17. 80591
  18. 106237
  19. 42933
  20. 2017574
  21. 27902
  22. 348140 (99.9 percentile)
  23. 11811
  24. 109830
  25. 51153
  26. 41510
  27. 8024
  28. 879
  29. 25566
  30. 400
  31. 3028
  32. 2366
  33. 664
  34. 335
  35. 208
  36. 1925
  37. 400
  38. 1874
  39. 131
  40. 67
  41. 119
  42. 20
  43. 216
  44. 82
  45. 205
  46. 952
  47. 2890
  48. 7078
  49. 54804 (99.999 percentile)
  50. 543
  51. 342 (99.9999 percentile)
  52. 21
  53. 8
  54. 288
  55. 0
  56. 102 (99.99999 percentile)
  57. 0
  58. 0
  59. 0
  60. 0
  61. 43 (100 percentile)

@Lichtso Lichtso added the feature-gate Pull Request adds or modifies a runtime feature gate label Sep 20, 2022
@Lichtso Lichtso changed the title Refactor/instruction context capacity Feature: Explicitly limit TransactionContext::instruction_trace_capacity Sep 20, 2022
@Lichtso Lichtso force-pushed the refactor/instruction_context_capacity branch 3 times, most recently from 7de1137 to 68bf5b2 Compare September 20, 2022 14:57
@Lichtso Lichtso requested a review from alessandrod September 20, 2022 16:55
@Lichtso Lichtso force-pushed the refactor/instruction_context_capacity branch from 68bf5b2 to 0582d68 Compare September 22, 2022 17:12
Copy link
Contributor

@alessandrod alessandrod left a comment

Choose a reason for hiding this comment

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

LGTM, but I think we should add a test for when the max is exceeded?

@alessandrod
Copy link
Contributor

Also, some downstream-project programs are failing since they hit the maximum, which is a hint that maybe it should be higher?

@Lichtso Lichtso force-pushed the refactor/instruction_context_capacity branch from 0582d68 to ddff44a Compare September 25, 2022 16:15
@Lichtso
Copy link
Contributor Author

Lichtso commented Sep 25, 2022

@alessandrod I fixed the tests in downstream-project and added a test for the new error.

@alessandrod alessandrod self-requested a review September 26, 2022 08:34
@Lichtso Lichtso merged commit 71aee4f into solana-labs:master Sep 26, 2022
@Lichtso Lichtso deleted the refactor/instruction_context_capacity branch September 26, 2022 08:47
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