Skip to content

Conversation

vtjnash
Copy link
Member

@vtjnash vtjnash commented Apr 21, 2025

The point of #57979 was to make inference faster, but it made it instead much slower (83524ac#commitcomment-155658124), so revert back to the fast behavior before it was "optimized" (and revert the bugfixes for the original commit).

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

@vtjnash
Copy link
Member Author

vtjnash commented Apr 21, 2025

@nanosoldier runbenchmarks("inference", vs="@92fc06fd83")

@nanosoldier
Copy link
Collaborator

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

@nanosoldier
Copy link
Collaborator

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

@vtjnash vtjnash added revert This reverts a previously merged PR. compiler:inference Type inference labels Apr 21, 2025
@vtjnash
Copy link
Member Author

vtjnash commented Apr 21, 2025

Looks like this is confirmed to fix the issues introduced by those PRs

@vtjnash vtjnash merged commit a7b8c83 into master Apr 22, 2025
9 of 11 checks passed
@vtjnash vtjnash deleted the jn/revert-57979-58083-58082 branch April 22, 2025 12:11
@aviatesk
Copy link
Member

/cc @serenity4 @Keno

Also this seems to require Cthulhu to be updated accordingly.
JuliaDebug/Cthulhu.jl#639

@aviatesk
Copy link
Member

Personally, I think the impact on the overall inference pipeline is limited, because these commits don't slow down the "allinference" benchmark set, only "abstract interpretation" (i.e the parts of inference up to generating unoptimized typed CodeInfo). So, I'm hoping we could try to get consensus again and see if we can merge these PRs.

@Keno
Copy link
Member

Keno commented Apr 23, 2025

Yes, can we please actually investigate things? These were not perf optimizations but API cleanups for downstream consumers (which are now all broken again). I'm sure there's just a missing case somewhere.

Keno added a commit that referenced this pull request Apr 23, 2025
Keno added a commit that referenced this pull request Apr 30, 2025
Reverts #58182. The API changes were intentional and
desirable. Let's figure out why nansoldier was upset and re-apply this.

---------

Co-authored-by: Cédric Belmant <[email protected]>
serenity4 pushed a commit to serenity4/julia that referenced this pull request May 1, 2025
JuliaLang#58182)

The point of JuliaLang#57979 was to make inference faster, but it made it instead
much slower
(JuliaLang@83524ac#commitcomment-155658124),
so revert back to the fast behavior before it was "optimized" (and
revert the bugfixes for the original commit).
serenity4 added a commit to serenity4/julia that referenced this pull request May 1, 2025
JuliaLang#58203)

Reverts JuliaLang#58182. The API changes were intentional and
desirable. Let's figure out why nansoldier was upset and re-apply this.

---------

Co-authored-by: Cédric Belmant <[email protected]>
LebedevRI pushed a commit to LebedevRI/julia that referenced this pull request May 2, 2025
JuliaLang#58182)

The point of JuliaLang#57979 was to make inference faster, but it made it instead
much slower
(JuliaLang@83524ac#commitcomment-155658124),
so revert back to the fast behavior before it was "optimized" (and
revert the bugfixes for the original commit).
charleskawczynski pushed a commit to charleskawczynski/julia that referenced this pull request May 12, 2025
JuliaLang#58203)

Reverts JuliaLang#58182. The API changes were intentional and
desirable. Let's figure out why nansoldier was upset and re-apply this.

---------

Co-authored-by: Cédric Belmant <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compiler:inference Type inference revert This reverts a previously merged PR.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants