smithy4s
smithy4s copied to clipboard
Improve cache in code generation
Solves part of #1495
PR Checklist (not all items are relevant to all PRs)
- [ ] Added unit-tests (for runtime code)
- [ ] Added bootstrapped code + smoke tests (when the rendering logic is modified)
- [ ] Added build-plugins integration tests (when reflection loading is required at codegen-time)
- [ ] Added alloy compliance tests (when simpleRestJson protocol behaviour is expanded/updated)
- [ ] Updated dynamic module to match generated-code behaviour
- [ ] Added documentation
- [ ] Updated changelog
Don't forget to sign the CLA
Since it was requested in Mill's chat room, here is some input regarding Mill's cache. I have to admit, I didn't review the full PR and all comments, but from what I digested, here are my notes:
-
If you return a
PathRef
that points to somewhere outside of Mill's cache, there is a chance Mill will miss it's later invalidation due to removal or change. Therefore in Mill 0.11 we introduced therevalidate
flag which you can enable withPathRef.withRevalidateOnce
. When enabled and before re-using a targets cache, Mill will re-calculate thePathRef.sig
and will only use the cached value if thesig
is still identical. I think you should use this flag since you don't have exclusive control over the output directory. -
Using an
T.input
or aPathRef
in general for a path you intend to change from a downstream target, will most likely cause unnecessary re-evaluation. Since you intend to generate something into that path, it'ssig
will change. If you're not interested in the content of a path, which is the case for config values, the path itself should be enough.
LGTM (@Baccata, you haven't committed here so I'll leave it for you to approve if, it looks OK now)