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

Implement performance optimizer #21

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

64kramsystem
Copy link
Collaborator

@64kramsystem 64kramsystem commented Oct 25, 2021

Super-simple™ implementation of the performance optimizer.

This is a per-instruction optimizer, which is in turn a simplified optimizer in itself. Future extension to multiple-instruction optimization will surely require a more extensive change.

There is a metric ton of design considerations. This PR can be considered a draft, or not :) The most important considerations are:

  • the Fisk::Instruction fields have been exposed; this is necessary, as the optimizer needs to know the instruction, and will need even more, in case it will work on multiple instructions; the alternative OO-design of adding an optimizer on the specific Fisk::Instructions::Instruction is bound to break down
  • the Optimizer could now absorb the performance check (performance checking is now located separately from performance optimization, which is not Vewy Nice); I think it requires a bit of design agreement (I have ideas about what to do), which can be done either in this PR or in a separate one.

Relates to tenderlove/tenderjit#49.

The `Temp@register` instance variable was not initialized, so it caused a warning in the build (where the error codepath is invoked).
Preparatory refactoring for the performance functionality(ies).
This is required by the upcoming optimizer.
@64kramsystem 64kramsystem added the enhancement New feature or request label Oct 25, 2021
@64kramsystem 64kramsystem self-assigned this Oct 25, 2021
@64kramsystem
Copy link
Collaborator Author

Ah, I've added a commit that cleans up a warning, avoiding a separate PR. If you prefer strict PRs, LMK, I have no problem with that approach 🙂.

@64kramsystem 64kramsystem force-pushed the performance_optimizer branch from 13d4cad to d898afc Compare October 25, 2021 22:47
@64kramsystem 64kramsystem marked this pull request as ready for review November 5, 2021 21:52
@64kramsystem
Copy link
Collaborator Author

@tenderlove Do I need to do anything on this PR? Ultimately, more significant changes (e.g. merging checking and optimization logic) can be done separately (or not done at all!).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant