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

Revert "Avoid allocations in views of views (#53231)" #57044

Merged
merged 1 commit into from
Jan 14, 2025

Conversation

KristofferC
Copy link
Member

@KristofferC KristofferC commented Jan 14, 2025

This reverts commit 2bd4cf8. (#53231)

The reason for this revert is that it caused exponential blowup of types in iterated views causing some packages to simply freeze when doing something that worked ok in 1.10.

In my opinion, the perf gain from the PR is not outweighed by the "risk" of hitting this compilation blowup case.

Fixes #56760.

@KristofferC KristofferC added the backport 1.10 Change should be backported to the 1.10 release label Jan 14, 2025
@KristofferC KristofferC requested a review from jishnub January 14, 2025 12:47
@KristofferC KristofferC added backport 1.11 Change should be backported to release-1.11 and removed backport 1.10 Change should be backported to the 1.10 release labels Jan 14, 2025
Copy link
Member

@jishnub jishnub left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree, this needs to be rethought.

@oscardssmith oscardssmith merged commit b23f557 into master Jan 14, 2025
11 checks passed
@oscardssmith oscardssmith deleted the kc/revert_alloc_view_avoid branch January 14, 2025 15:28
KristofferC added a commit that referenced this pull request Jan 14, 2025
This reverts commit 2bd4cf8. (#53231)

The reason for this revert is that it caused exponential blowup of types
in iterated views causing some packages to simply freeze when doing
something that worked ok in 1.10.

In my opinion, the perf gain from the PR is not outweighed by the "risk"
of hitting this compilation blowup case.

Fixes #56760.

Co-authored-by: KristofferC <[email protected]>
(cherry picked from commit b23f557)
@KristofferC KristofferC removed the backport 1.11 Change should be backported to release-1.11 label Jan 28, 2025
Zentrik referenced this pull request Feb 8, 2025
Allow for more spaces between the "Total" and "Time" columns, in case tests
take occasionally longer than usual.

Fix #57095.
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.

Iterative views leads to nested SubArray type blow-up
3 participants