Skip to content

IRGen: Only emit PragmaCommentDecls if building for Windows, or LTO is enabled #35747

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

Conversation

slavapestov
Copy link
Contributor

@slavapestov slavapestov commented Feb 3, 2021

clang::DeclContext::decls_begin() deserializes all the pre-compiled headers
imported into the current translation unit, which is expensive.

Workaround this by disabling the emission of PragmaCommentDecls unless we're
building for a Windows target, or LTO is enabled.

A better fix would be to serialize a separate index for PragmaCommentDecls
so that they can be deserialized without deserializing everything else.
I filed rdar://problem/74036099 to track the longer-term fix.

Fixes rdar://problem/73951264.

@slavapestov slavapestov force-pushed the irgen-clang-deserialization-regression branch from e1b4fb2 to ac16f90 Compare February 3, 2021 23:07
@slavapestov
Copy link
Contributor Author

@martinboehme Does this look like a reasonable fix to you?

@slavapestov
Copy link
Contributor Author

Ok, this breaks the lto_autolink test unfortunately. @kateinoigakukun any ideas how we can get this information without deserializing everything?

@kateinoigakukun
Copy link
Member

Ok, this breaks the lto_autolink test unfortunately. @kateinoigakukun any ideas how we can get this information without deserializing everything?

Sorry I have no idea how to get pragmas without loading all decls.

@slavapestov slavapestov force-pushed the irgen-clang-deserialization-regression branch from ac16f90 to 29ef1d7 Compare February 5, 2021 20:34
…s enabled

clang::DeclContext::decls_begin() deserializes all the pre-compiled headers
imported into the current translation unit, which is expensive.

Workaround this by disabling the emission of PragmaCommentDecls unless we're
building for a Windows target, or LTO is enabled.

A better fix would be to serialize a separate index for PragmaCommentDecls
so that they can be deserialized without deserializing everything else.
I filed rdar://problem/74036099 to track the longer-term fix.

Fixes rdar://problem/73951264.
@slavapestov slavapestov force-pushed the irgen-clang-deserialization-regression branch from 29ef1d7 to c0aa202 Compare February 5, 2021 20:35
@slavapestov slavapestov changed the title IRGen: Use clang::DeclContext::noload_decls() in place of ...::decls() IRGen: Only emit PragmaCommentDecls if building for Windows, or LTO is enabled Feb 5, 2021
@slavapestov
Copy link
Contributor Author

@kateinoigakukun @martinboehme I changed the fix to narrow it down to Windows and LTO. This makes the existing tests pass while recovering the performance regression in the common case.

@slavapestov
Copy link
Contributor Author

@swift-ci Please smoke test

@slavapestov slavapestov merged commit 1ae38f8 into swiftlang:main Feb 6, 2021
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.

None yet

3 participants