Skip to content

[5.5] In DI, cache whether a memory object is a box #37329

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

Merged
merged 1 commit into from
May 10, 2021

Conversation

rjmccall
Copy link
Contributor

@rjmccall rjmccall commented May 7, 2021

Boxes tend to have a small number of uses, so frequently finding the unique projection isn't too bad. Non-boxes, however, can have a large number of uses: for example, a class instance has expected uses proportionate to the number of stored properties. So if we do a linear scan of the uses on a non-box instruction, we'll scale quadratically.

Fixes SR-14532. 5.5 version of #37328

Explanation: Avoid a quadratic slowdown in DI in class initializers when a class reference has many uses. This is typical when a class has a large number of properties.
Scope: The change is to Swift’s definitive initialization pass, which is run on all functions
Risk: Low
Testing: Manual; no straightforward way to test future regressions here
Issue: rdar://77217125
Reviewer: Erik Eckstein

@rjmccall rjmccall requested a review from a team as a code owner May 7, 2021 22:54
Boxes tend to have a small number of uses, so frequently finding the
unique projection isn't too bad.  Non-boxes, however, can have a large
number of uses: for example, a class instance has expected uses
proportionate to the number of stored properties.  So if we do a linear
scan of the uses on a non-box instruction, we'll scale quadratically.

Fixes SR-14532.
@rjmccall rjmccall force-pushed the di-box-scaling-5.5 branch from 657f3e8 to b8e8d2e Compare May 7, 2021 22:54
@rjmccall
Copy link
Contributor Author

rjmccall commented May 7, 2021

@swift-ci Please test

@rjmccall rjmccall requested review from eeckstein and gottesmm May 8, 2021 00:39
Copy link
Contributor

@eeckstein eeckstein left a comment

Choose a reason for hiding this comment

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

LGTM!

As a side note: now that we have a clean way to represent multi-value instructions we can consider removing project_box again and let alloc_box return two values.

@tkremenek tkremenek merged commit 1260368 into swiftlang:release/5.5 May 10, 2021
@rjmccall rjmccall deleted the di-box-scaling-5.5 branch May 10, 2021 22:00
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.

3 participants