Topic/jit optimize #5348
Merged
Topic/jit optimize #5348
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- Apparently `(max 0 n)` used in `Nat.drop` was slow, so it's been replaced with something that should act the same on natural numbers. - Switched back to the original currying macro behavior. This seems to optimize better in various ways. According to my tests, it should only really be necessary for recursive functions, and so I've added some capabilities to only apply the full macro locally on those. But the racket optimizer also seems very fickle, so using predefined curry functions on various builtins seems to _not_ optimize properly like they do in my localized tests, even when various inlining suggestions are enabled. Hopefully this can be fixed in the future as it makes compile times significantly worse. This also fixes a latent bug where there wouldn't be enough pre-defined currying functions for procedures that take more than 20 arguments. I've instead lowered the predefined functions to a maximum of 9 arguments, and made anything over that just use the macro directly, since those are presumably rare. None of the currying functions are currently used, but hopefully they can be in the future.
This pull request was closed.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR has some quick stuff to make some arithmetic work faster on the JIT. There's more work to do, but this is a first step at least.
One change is that using
(max 0 ...)in the implementation ofNat.dropwas slow, so if you use that in a loop, you lose performance. It's been replaced with something that should be equivalent, but faster.I've also changed the way that unison definitions get 'curried.' It's back to the original strategy of generating
case-lambdaexpressions for every definition. My experiments suggest that this optimizes better in various cases. I've also added machinery to selectively apply this behavior, because it causes compilation to be a lot slower.According to my tests, it shouldn't be necessary for every definition to use this strategy. It's mostly recursive functions that the optimizer refuses to handle well with pre-defined currying functions. But I also couldn't get the optimizer to optimize builtins properly in actual code without them also using this sort of currying. At this point I'm unsure of what the difference between my test cases and the actual code is, so I thought I'd just push this to get the optimization out, and try to figure out how to be more intelligent about it later.
With this, counting up to 1 billion takes around 1.5s on my machine, which matches a loop written directly in racket. This is only testing a couple operations, though, so there may be random other things like the
(max 0 ...)situation out there that I haven't looked at yet.