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

allow destructive inlining only when the source is volatile #52062

Merged
merged 1 commit into from
Nov 8, 2023

Conversation

aviatesk
Copy link
Member

@aviatesk aviatesk commented Nov 7, 2023

Destructive inlining introduced by #51934 implicitly presupposes that inferred CodeInfo source has been compressed to the String format when cached, which is not generally true for external AbstractInterpreters that may disable the may_compress and/or may_discard_trees settings. This commit adds a safeguard to ensure the eligibility of such CodeInfo source propagated by VolatileInferenceResult for destructive inlining.

@nanosoldier runbenchmarks("inference", vs=":master")

@nanosoldier
Copy link
Collaborator

Your benchmark job has completed - no performance regressions were detected. A full report can be found here.

Base automatically changed from avi/rm-must_be_codeinf to master November 8, 2023 00:47
Destructive inlining introduced by #51934 implicitly
presupposes that inferred `CodeInfo` source has been compressed to the
`String` format when cached, which is not generally true for external
`AbstractInterpreter`s that may disable the `may_compress`
and/or `may_discard_trees` settings. This commit adds a safeguard to
ensure the eligibility of such `CodeInfo` source propagated by
`VolatileInferenceResult` for destructive inlining.
@aviatesk aviatesk force-pushed the avi/is_src_volatile branch from 7c66928 to 1ec2ad0 Compare November 8, 2023 00:48
@aviatesk aviatesk merged commit 1327dfe into master Nov 8, 2023
@aviatesk aviatesk deleted the avi/is_src_volatile branch November 8, 2023 00:51
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.

2 participants