Workaround for stack overflow in stream refine usage. #20749
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
On certain models (like that in #19832) kvcache updates end up as massively long sequences of tied dispatches. The attributor-style analysis uses recursion to initialize elements on first use. Tied ops need to walk through the operand to the result and transitively to further uses. When their powers combine we get stack overflows. There are a few ways around this but this change to avoid update is the simplest. It's not a great solution as it may lead to more iterations as values initialized during ValueResourceUsage update will require at least one iteration to perform their initial update and then one more to quiesce. The best solution would be to avoid recursion by changing the API for querying elements but that's a much larger bit of refactoring. If this does cause issues (like major regressions in compile times) there's probably some other ways that could be explored but here's to hoping no one notices anything. history of reverts if one is required here).
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.
On certain models (like that in #19832) kvcache updates end up as massively long sequences of tied dispatches. The attributor-style analysis uses recursion to initialize elements on first use. Tied ops need to walk through the operand to the result and transitively to further uses. When their powers combine we get stack overflows.
There are a few ways around this but this change to avoid update is the simplest. It's not a great solution as it may lead to more iterations as values initialized during ValueResourceUsage update will require at least one iteration to perform their initial update and then one more to quiesce. The best solution would be to avoid recursion by changing the API for querying elements but that's a much larger bit of refactoring. If this does cause issues (like major regressions in compile times) there's probably some other ways that could be explored but here's to hoping no one notices anything.
(filed #20748 to track the history of reverts if one is required here).