Skip to content

Remove a hack to change linkage from public_external to shared. #9736

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 18, 2017

Conversation

eeckstein
Copy link
Contributor

Explanation: This change fixes a big code duplication problem. The compiler generated code for public functions which are imported from the stdlib - and are also available in the swiftCore library.
This got worse since we use public linkage for @_versioned internal functions in the stdlib.
The code size overhead by this duplication is up to 40% for some projects. Which means that by fixing this, we get up to 40% code size reduction.
There is no significant impact on benchmark performance.

Scope: This problem has a big effect on the code size.

Radar: rdar://problem/32265845

Risk: Low. It has no effect on generated code (it only removes duplicated functions). And it has no effect on generated swiftmodule files.

Testing: There is a specific test for swift-ci. Also it's implicitly tested by all tests/projects which link an executable.

The linkage change let the compiler generate code for public functions which are imported from the stdlib - and are also available in the swiftCore library.
This got worse since we use public linkage for @_versioned internal functions in the stdlib.

Getting rid of the linkage change reduces code size a lot: up to 40% for some projects.

I didn’t see any significant impact on benchmark performance.
@eeckstein
Copy link
Contributor Author

@swift-ci test

@tkremenek tkremenek merged commit 552c833 into swiftlang:swift-4.0-branch May 18, 2017
@eeckstein eeckstein deleted the remove-linkage-hack branch May 18, 2017 22:16
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