Skip to content

Conversation

grynspan
Copy link
Contributor

@grynspan grynspan commented Oct 2, 2024

This PR turns off automatic reformatting of our macro expansions. I noticed that at-desk, the compiler was spending an inordinate amount of time in reformatting:

Call graph: 7149 Thread_709950 DispatchQueue_1: com.apple.main-thread (serial) 7149 start (in dyld) + 2840 [0x18dafc274] 7149 main (in TestingMacros) + 28 [0x1029291d4] TestingMacrosMain.swift:18 7149 static TestingMacrosMain.$main() (in TestingMacros) + 160 [0x1029290c4] /<compiler-generated>:0 7149 static CompilerPlugin.main() (in TestingMacros) + 544 [0x10337f64c] CompilerPlugin.swift:115 6702 CompilerPluginMessageListener.main() (in TestingMacros) + 536 [0x1033cc85c] CompilerPluginMessageHandler.swift:96 + 6692 CompilerPluginMessageHandler.handleMessage(_:) (in TestingMacros) + 2320 [0x1033ce32c] CompilerPluginMessageHandler.swift:169 + ! 6375 CompilerPluginMessageHandler.expandAttachedMacro(macro:macroRole:discriminator:attributeSyntax:declSyntax:parentDeclSyntax:extendedTypeSyntax:conformanceListSyntax:lexicalContext:) (in TestingMacros) + 1572 [0x1033fec00] Macros.swift:155 + ! : 3284 expandAttachedMacroWithoutCollapsing<A>(definition:macroRole:attributeNode:declarationNode:parentDeclNode:extendedType:conformanceList:in:indentationWidth:) (in TestingMacros) + 3132 [0x1033a96d0] MacroExpansion.swift:280 + ! : | 3284 Collection.map<A, B>(_:) (in TestingMacros) + 1656 [0x1033a7eec] /<compiler-generated>:0 + ! : | 3284 partial apply for closure #4 in expandAttachedMacroWithoutCollapsing<A>(definition:macroRole:attributeNode:declarationNode:parentDeclNode:extendedType:conformanceList:in:indentationWidth:) (in TestingMacros) + 36 [0x1033ac83c] /<compiler-generated>:0 + ! : | 3284 closure #4 in expandAttachedMacroWithoutCollapsing<A>(definition:macroRole:attributeNode:declarationNode:parentDeclNode:extendedType:conformanceList:in:indentationWidth:) (in TestingMacros) + 180 [0x1033aaaa8] MacroExpansion.swift:281 + ! : | 3243 SyntaxProtocol.formattedExpansion(_:indentationWidth:) (in TestingMacros) + 188 [0x1033a81f4] MacroExpansion.swift:409 

3243 samples is significantly more than what we spend in test macro expansion:

+ ! : 2277 expandAttachedMacroWithoutCollapsing<A>(definition:macroRole:attributeNode:declarationNode:parentDeclNode:extendedType:conformanceList:in:indentationWidth:) (in TestingMacros) + 2932 [0x1033a9608] MacroExpansion.swift:273 + ! : | 2277 protocol witness for static PeerMacro.expansion<A, B>(of:providingPeersOf:in:) in conformance TestDeclarationMacro (in TestingMacros) + 20 [0x102928ad0] /<compiler-generated>:0 + ! : | 2258 static TestDeclarationMacro.expansion<A, B>(of:providingPeersOf:in:) (in TestingMacros) + 316 [0x10291f73c] TestDeclarationMacro.swift:31 + ! : | + 966 static TestDeclarationMacro._createTestContainerDecls<A>(for:on:testAttribute:in:) (in TestingMacros) + 2184 [0x1029213b8] TestDeclarationMacro.swift:410 

These samples are in a debug build—I fully expect the performance to be better
in a release build—but they still significantly impact our build time when used
as a package or tested at desk.

Works around rdar://137132570.

Checklist:

  • Code and documentation should follow the style of the Style Guide.
  • If public symbols are renamed or modified, DocC references should be updated.
This PR turns off automatic reformatting of our macro expansions. I noticed that at-desk, the compiler was spending an inordinate amount of time in reformatting: ``` Call graph: 7149 Thread_709950 DispatchQueue_1: com.apple.main-thread (serial) 7149 start (in dyld) + 2840 [0x18dafc274] 7149 main (in TestingMacros) + 28 [0x1029291d4] TestingMacrosMain.swift:18 7149 static TestingMacrosMain.$main() (in TestingMacros) + 160 [0x1029290c4] /<compiler-generated>:0 7149 static CompilerPlugin.main() (in TestingMacros) + 544 [0x10337f64c] CompilerPlugin.swift:115 6702 CompilerPluginMessageListener.main() (in TestingMacros) + 536 [0x1033cc85c] CompilerPluginMessageHandler.swift:96 + 6692 CompilerPluginMessageHandler.handleMessage(_:) (in TestingMacros) + 2320 [0x1033ce32c] CompilerPluginMessageHandler.swift:169 + ! 6375 CompilerPluginMessageHandler.expandAttachedMacro(macro:macroRole:discriminator:attributeSyntax:declSyntax:parentDeclSyntax:extendedTypeSyntax:conformanceListSyntax:lexicalContext:) (in TestingMacros) + 1572 [0x1033fec00] Macros.swift:155 + ! : 3284 expandAttachedMacroWithoutCollapsing<A>(definition:macroRole:attributeNode:declarationNode:parentDeclNode:extendedType:conformanceList:in:indentationWidth:) (in TestingMacros) + 3132 [0x1033a96d0] MacroExpansion.swift:280 + ! : | 3284 Collection.map<A, B>(_:) (in TestingMacros) + 1656 [0x1033a7eec] /<compiler-generated>:0 + ! : | 3284 partial apply for closure #4 in expandAttachedMacroWithoutCollapsing<A>(definition:macroRole:attributeNode:declarationNode:parentDeclNode:extendedType:conformanceList:in:indentationWidth:) (in TestingMacros) + 36 [0x1033ac83c] /<compiler-generated>:0 + ! : | 3284 closure #4 in expandAttachedMacroWithoutCollapsing<A>(definition:macroRole:attributeNode:declarationNode:parentDeclNode:extendedType:conformanceList:in:indentationWidth:) (in TestingMacros) + 180 [0x1033aaaa8] MacroExpansion.swift:281 + ! : | 3243 SyntaxProtocol.formattedExpansion(_:indentationWidth:) (in TestingMacros) + 188 [0x1033a81f4] MacroExpansion.swift:409 ` 3243 samples is significantly more than what we spend in test macro expansion: ``` + ! : 2277 expandAttachedMacroWithoutCollapsing<A>(definition:macroRole:attributeNode:declarationNode:parentDeclNode:extendedType:conformanceList:in:indentationWidth:) (in TestingMacros) + 2932 [0x1033a9608] MacroExpansion.swift:273 + ! : | 2277 protocol witness for static PeerMacro.expansion<A, B>(of:providingPeersOf:in:) in conformance TestDeclarationMacro (in TestingMacros) + 20 [0x102928ad0] /<compiler-generated>:0 + ! : | 2258 static TestDeclarationMacro.expansion<A, B>(of:providingPeersOf:in:) (in TestingMacros) + 316 [0x10291f73c] TestDeclarationMacro.swift:31 + ! : | + 966 static TestDeclarationMacro._createTestContainerDecls<A>(for:on:testAttribute:in:) (in TestingMacros) + 2184 [0x1029213b8] TestDeclarationMacro.swift:410 ``` These samples are in a debug build—I fully expect the performance to be better in a release build—but they still significantly impact our build time when used as a package or tested at desk. Works around rdar://137132570.
@grynspan grynspan added bug 🪲 Something isn't working workaround Workaround for an issue in another component (may need to revert later) performance 🏎️ Performance issues labels Oct 2, 2024
@grynspan grynspan self-assigned this Oct 2, 2024
@grynspan
Copy link
Contributor Author

grynspan commented Oct 2, 2024

@swift-ci test

@grynspan
Copy link
Contributor Author

grynspan commented Oct 2, 2024

@swift-ci test

@grynspan grynspan merged commit 5763213 into main Oct 2, 2024
3 checks passed
@grynspan grynspan deleted the jgrynspan/no-reformatting-of-macro-expansions branch October 2, 2024 18:08
grynspan added a commit that referenced this pull request Oct 2, 2024
Follow-on to #743. Disables formatting for `#_sourceLocation` and `@Tag`. Although I haven't seen either in a sample/spindump, every little bit counts and neither of these macros needs the formatting to build correctly.
@grynspan grynspan mentioned this pull request Oct 2, 2024
2 tasks
grynspan added a commit that referenced this pull request Oct 3, 2024
Follow-on to #743. Disables formatting for `#_sourceLocation` and `@Tag`. Although I haven't seen either in a sample/spindump, every little bit counts and neither of these macros needs the formatting to build correctly. ### Checklist: - [x] Code and documentation should follow the style of the [Style Guide](https://github.com/apple/swift-testing/blob/main/Documentation/StyleGuide.md). - [x] If public symbols are renamed or modified, DocC references should be updated.
@grynspan grynspan added this to the Swift 6.1 milestone Oct 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug 🪲 Something isn't working performance 🏎️ Performance issues workaround Workaround for an issue in another component (may need to revert later)

2 participants