Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
[FrameworkBundle] Dont wire "annotations.cached_reader" before removing passes#21381
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
fabpot commentedJan 23, 2017
Is it possible to some tests? Because I fear we can get regressions on this one. |
nicolas-grekas commentedJan 24, 2017
Test added. Without the patch, the test suite fails with many |
fabpot commentedJan 24, 2017
Thank you@nicolas-grekas. |
…fore removing passes (nicolas-grekas)This PR was merged into the 3.2 branch.Discussion----------[FrameworkBundle] Dont wire "annotations.cached_reader" before removing passes| Q | A| ------------- | ---| Branch? | 3.2| Bug fix? | yes| New feature? | no| BC breaks? | no| Deprecations? | no| Tests pass? | yes| Fixed tickets |#21339| License | MIT| Doc PR | -This PR basically reverts#20601 and wires "annotations.cached_reader" later, so that any compiler passes needing "annotation_reader" at compile time don't get any cache - anyway, it's useless at compile time.Commits-------e59f0e0 [FrameworkBundle] Dont wire "annotations.cached_reader" before removing passes
stof commentedJan 24, 2017
this is wrong, as it changes the object graph after optimizations, which may change the allowed inlining. Instead it should stay a pass registered before optimizations, but with a very low priority |
stof commentedJan 24, 2017
in practice, adding more compiler passes after optimization should be quite rare, as changes done in them may break previous optimizations |
nicolas-grekas commentedJan 24, 2017
Not sure to get what you mean. To me, the graph is computed in removing passes, by AnalyzeServiceReferencesPass, so this won't be an issue. |
… bootstrap (nicolas-grekas)This PR was merged into the 3.2 branch.Discussion----------[FrameworkBundle] Wire ArrayCache for annotation reader at bootstrap| Q | A| ------------- | ---| Branch? | 3.2| Bug fix? | yes| New feature? | no| BC breaks? | no| Deprecations? | no| Tests pass? | yes| Fixed tickets |#21549| License | MIT| Doc PR | -Related to#21381 which disabled any cache at bootstrap.Instead, this wires ArrayCache during bootstrapping, then swaps the cache provider for the real one.Commits-------f90f53e [FrameworkBundle] Wire ArrayCache for annotation reader at bootstrap
…ss (nicolas-grekas)This PR was merged into the 3.4 branch.Discussion----------[FrameworkBundle] remove dead code in CachePoolClearerPass| Q | A| ------------- | ---| Branch? | 3.4| Bug fix? | no| New feature? | no| BC breaks? | no| Deprecations? | no| Tests pass? | yes| Fixed tickets | -| License | MIT| Doc PR | -This code had been removed in#21381, I don't know why it's still there but this is dead code, as the `cache.annotations` service inherits a logger from the parent `cache.system` service.Commits-------974991f [FrameworkBundle] remove dead code in CachePoolClearerPass
Uh oh!
There was an error while loading.Please reload this page.
This PR basically reverts#20601 and wires "annotations.cached_reader" later, so that any compiler passes needing "annotation_reader" at compile time don't get any cache - anyway, it's useless at compile time.