Skip to content

Optimize schedule.md#2335

Closed
dwightguth wants to merge 5 commits intomasterfrom
schedule
Closed

Optimize schedule.md#2335
dwightguth wants to merge 5 commits intomasterfrom
schedule

Conversation

@dwightguth
Copy link
Copy Markdown
Contributor

@dwightguth dwightguth commented Mar 6, 2024

Previously we were implementing the gas schedule in schedule.md in a fairly modular way, with each schedule defined in terms of its difference from the previous hard fork. As the number of hard forks in KEVM has increased, however, this has gradually become a significant performance cost, because the most common case is the evaluation of the gas cost of opcodes that have not changed in cost since FRONTIER, as a result of which, we end up repeatedly evaluating rules that tell us that the gas cost can be looked up in a previous schedule.

To optimize this, we change schedule.md so that we directly emit each gas schedule to each hard fork so that the gas cost of any operand can be looked up directly using pattern matching on the arguments. The resulting code is slightly less modular, but it also has the advantage of being somewhat more readable; it is easy to look up the exact cost of any constant in any given schedule directly in schedule.md without having to trace back and determine its exact value by following through all the forks of EVM.

There is one additional benefit to this approach: in later gas schedules, some of the constants in the schedule were defined in terms of other constants. However, the way this had been implemented in KEVM was such that if the dependent constant changed in a future hard fork, we would still have to update both constants in schedule.md. This gave the appearance of increased modularity, when in fact no such modularity actually existed. This change makes it much more apparent that such modularity does not in fact exist, thus decreasing the likelihood of bugs slipping through as a result of lack of test coverage when implementing a future hard fork.

There is one additional change that we make in this PR: we delete the DEFAULT schedule. That schedule was essentially an alias for HOMESTEAD, but was deceptively named. Thus we replace DEFAULT everywhere explicitly with HOMESTEAD.

@dwightguth dwightguth marked this pull request as ready for review March 7, 2024 18:30
@ehildenb
Copy link
Copy Markdown
Member

ehildenb commented Mar 7, 2024

Is it possible to instead use rule priorities to optimize the schedule instead? Would that yield the same performance benefit? Then, we wouldn't have as much duplication of the logic there.

@ehildenb
Copy link
Copy Markdown
Member

ehildenb commented Mar 7, 2024

Another option would be to implement each schedule option/flag once, but wiht #ite on the RHS for any changes between different schedules. This may require defining a comparator _<=_: Schedule Schedule -> bool for that to work, to put an explicit order on schedules.

@dwightguth
Copy link
Copy Markdown
Contributor Author

I'm not sure how you would use rule priorities to implement a constant-time algorithm here. At most you might save some of the time spent evaluating side conditions, but you would still end up having to recursively invoke the schedule operator a linear number of times in the number of hard forks. Even with this optimization, this function is a hotspot in the performance profile, so I don't think it's a good idea to keep a linear-time algorithm here, especially as the number of hard forks is only going to keep increasing.

Using an ite is a little bit better, but not much better because we lose the ability to encode the switches generated by the pattern matcher into a jump table. In theory, this function ought to be able to be executed with just two integer comparisons and a couple jump instructions. You're not going to be able to achieve that by means of an if/else tree.

@ehildenb
Copy link
Copy Markdown
Member

ehildenb commented Mar 7, 2024

Yeah, I figure that encoding it via the priorities will be faster than via function evaluation simply because it's built into the matcher directly. But who knows? Let me know how the experiment goes!

@dwightguth dwightguth closed this Mar 8, 2024
@dwightguth dwightguth mentioned this pull request Mar 8, 2024
@PetarMax
Copy link
Copy Markdown
Contributor

PetarMax commented Mar 9, 2024

I don't think that there is a major issue with maintaining all of the information explicitly per schedule, as @dwightguth suggests. The values should never be changed for previous schedules, and can easily be added for new schedules as well, by copying the values for the previous schedule and going from there. There is repetition, yes, but there is also clarity.

@dwightguth dwightguth mentioned this pull request Mar 12, 2024
dwightguth pushed a commit to fastxyz/evm-semantics that referenced this pull request Apr 15, 2024
dwightguth pushed a commit to fastxyz/evm-semantics that referenced this pull request Jun 17, 2024
dwightguth pushed a commit to fastxyz/evm-semantics that referenced this pull request Jun 18, 2024
dwightguth pushed a commit that referenced this pull request Jun 18, 2024
dwightguth pushed a commit that referenced this pull request Jun 25, 2024
Robertorosmaninho pushed a commit to fastxyz/evm-semantics that referenced this pull request Jul 5, 2024
Robertorosmaninho pushed a commit to fastxyz/evm-semantics that referenced this pull request Jul 9, 2024
Robertorosmaninho pushed a commit to fastxyz/evm-semantics that referenced this pull request Jul 9, 2024
dwightguth pushed a commit to fastxyz/evm-semantics that referenced this pull request Jul 9, 2024
Robertorosmaninho pushed a commit to fastxyz/evm-semantics that referenced this pull request Aug 7, 2024
Robertorosmaninho pushed a commit to fastxyz/evm-semantics that referenced this pull request Aug 13, 2024
Robertorosmaninho pushed a commit to fastxyz/evm-semantics that referenced this pull request Sep 12, 2024
dwightguth pushed a commit to fastxyz/evm-semantics that referenced this pull request Sep 25, 2024
Robertorosmaninho pushed a commit to fastxyz/evm-semantics that referenced this pull request Jan 28, 2025
Robertorosmaninho pushed a commit to fastxyz/evm-semantics that referenced this pull request Jan 31, 2025
Robertorosmaninho pushed a commit to fastxyz/evm-semantics that referenced this pull request Jan 31, 2025
Robertorosmaninho pushed a commit to fastxyz/evm-semantics that referenced this pull request Jan 31, 2025
Robertorosmaninho pushed a commit to fastxyz/evm-semantics that referenced this pull request Feb 10, 2025
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