Skip to content

Conversation

@ahoppen
Copy link
Member

@ahoppen ahoppen commented Dec 6, 2024

The transform to get the transformed result might be expensive, so we should cache its result.

@ahoppen
Copy link
Member Author

ahoppen commented Dec 6, 2024

@swift-ci Please test

Comment on lines 43 to 46
let transformed = Task { try await transform(value.value) }
// Cache the transformed result.
storage[key] = transformed
return try await transformed.value
Copy link
Contributor

Choose a reason for hiding this comment

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

Won't this re-apply the transform for every subsequent access?

Copy link
Member Author

Choose a reason for hiding this comment

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

Indeed, this was wrong on so many different levels. Thanks for catching it in the review, this would have been a pain to debug.

@ahoppen
Copy link
Member Author

ahoppen commented Dec 6, 2024

@swift-ci Please test

@ahoppen
Copy link
Member Author

ahoppen commented Dec 6, 2024

@swift-ci Please test Windows

The transform to get the transformed result might be expensive, so we should cache its result.
@ahoppen
Copy link
Member Author

ahoppen commented Dec 6, 2024

@swift-ci Please test

@ahoppen ahoppen enabled auto-merge December 6, 2024 19:59
@ahoppen
Copy link
Member Author

ahoppen commented Dec 6, 2024

@swift-ci Please test Windows

@ahoppen ahoppen merged commit fad3c10 into swiftlang:main Dec 7, 2024
3 checks passed
@ahoppen ahoppen deleted the cache-transformed branch August 4, 2025 10:48
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