[NFC] Clean up package graph caching code #7128
Conversation
This protocol doesn't define any requirements and was used for nemaspacing a single tuple typealias. Instead of typealiasing this fairly complex and pervasively used tuple, we should define a separate struct for it. Then the protocol itself can be removed as completely unused. Also used this as an opportunity to clean up file naming and directory layout in the `PackageGraph` module.
`Resolved*` types in the `PackageGraph` module were confusingly named as `Resolved*Builder`, while they had no relation to [the established builder pattern](https://www.swiftbysundell.com/articles/using-the-builder-pattern-in-swift/) in Swift. In fact, these types are doing only memoization, although proxying a few properties to initializer calls. The impact of this proxying remains to be benchmark, as I have suspicion that these types specifically don't bring a significant speed up, but only introduce a redundant level of indirection. In the meantime I've renamed these types to explicitly mention memoization and moved them out to a separate directory.
MaxDesiatov
added
the
no functional change
|
👍 this is a good change! also agree these types add complexity and are is not necessarily adding value. one thought, should we consider calling them "CachedXxx" instead of "MemoizedXxx"? |
MaxDesiatov
changed the title
|
Closing as superseded by #7160. After refactoring this code I found that in a convoluted way these actually are "builder" types, but they rely on multiple side effects of other reference types in They defer package graph construction until all of the side effects (including module aliasing) are applied, after which actual MO these types should be just plainly removed, but only after these side effects are cleaned up, otherwise we'd break module aliasing, which relies on reference types semantics and side effects. |
Motivation:
Depends on #7127.
Resolved*types in thePackageGraphmodule were confusingly named asResolved*Builder, while they had no relation to the established builder pattern in Swift. In fact, these types are doing only caching, although proxying a few properties to initializer calls. The impact of this proxying remains to be benchmarked, as I have suspicion that these types specifically don't bring a significant speed up, but only introduce a redundant level of indirection.Modifications:
I've renamed these types to explicitly mention caching and moved them out to a separate directory.
Result:
It's easier to understand that caching is being used. Relevant caching types are easier to find.