type-contract: merge vector and hash types earlier #1282
Merged
Conversation
This file contains hidden or 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
Context: (Vector T) is implemented as the union of two types. Enforcing the type with two contracts is inefficient; we want one merged contract. Same for (HashTable K V), which is implemented as three types. This commit replaces the old static-contract merging with a type merging. The problem with static contract merging is that it happens after the "typed-side" optimization kicks in --- which meant that two types with equal components could end up as static contracts with unequal components. This was a huge practical problem in the GTP benchmark `dungeon` ... the fix lowers the runtime for one loop in the worst case down from 30 seconds to 1/2 second.
|
Any suggestions for how to write regression tests? I'm thinking the unit tests for static contracts need a way to check expected output. |
capfredf
reviewed
Sep 10, 2022
we could extend |
jackfirth
added a commit
that referenced
this pull request
Sep 12, 2022
Just touching this file to get Resyntax to analyze it so I can debug why it didn't review #1282 properly.
Closed
bennn
commented
Sep 14, 2022
| [_ #false])) | ||
|
|
||
| (define (group-types-by-elems t->elems ts) | ||
| (group-by t->elems ts non-#f-and-equal?)) |
There was a problem hiding this comment.
Here & below, the code is more flexible that it needs to be. It lets t->elems return #f and calls the normal (t->sc _) if that happens.
If we raise an error instead of allowing #f, then we can delete this "group-types" function and one argument from merge-types->scs
capfredf
approved these changes
Sep 19, 2022
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.
Context:
(Vector T) is implemented as the union of two types. Enforcing the
type with two contracts is inefficient; we want one merged contract.
Same for (HashTable K V), which is implemented as three types.
This commit replaces the old static-contract merging with a type merging.
The problem with static contract merging is that it happens after the "typed-side" optimization kicks in --- which meant that two types with equal components could end up as static contracts with unequal components. This was a huge practical problem in the GTP benchmark
dungeon... the fix lowers the runtime for one loop in the worst case down from 30 seconds to 1/2 second.