cadence
cadence copied to clipboard
onflow
[LS] Better cache invalidation for imported program checkers
Issue To Be Solved
The language server creates a new checker every time when checking the imported programs as seen here, this might be quite expensive and was done as a quick patch to a problem in production.
Suggested Solution
The caching should be reverted and stay in place but there should be a cache invalidation system that is smarter, potentially by analyzing an inversed dependency tree and just invalidating the dependencies that changed.
Removing cached checkers caused an issue in LS and its consumers, where a certain type is not equal to the same type if imported in different location. This is probably coming from the fact that checker is now freshly created everytime a diagnostic is run which causes a new AST being built. Working on writing a base test suite that will detect this bug and also the previous bug with cached checkers not detecting changes and after the tests are in place work on a solution.
Base test suite indicates changes are processed correctly. Trying to set up the playground version at the time the issue was reported. Coming across some difficulties setting up that version.
@sideninja IDK if you saw, I opened #1634, which could be used as a foundation for the cache invalidation in the LS
I see everything 👁️ 😄 yes I did and thank you for that. It will serve well with this for sure. I'm working today to reproduce the bug locally with local playground setup. I want to make sure this is indeed causing the bug since when I now wrote tests it seemed like the cached checkers work ok.
This recording (sorry for potato resolution) is showing that the bug with added members is not possible to recreate. I’ve chosen the version before any updates and even though there are bunch of other issues with UI (a lot of them are now fixed) the one with member not being visible is not reproducable by me. I was monitoring the LS communication and the responses are correct. This makes me conclude it probably isn’t a bug related to LS. I will make sure to extend tests in LS potentially to find any problems, but the base test I made for this problem is working which is another indication it’s not a problem from the LS. cc @turbolent @srinjoyc
https://user-images.githubusercontent.com/75445744/167633602-4ff07673-2dfb-4689-9d15-4d949617e3c6.mov
Moving back to the backlog as this will be revisited during the other LS work.
@sideninja @turbolent any news on this issue? Making changes in a contract and redeploying does not reflect in the types imported by other accounts or transaction type checkers - pretty much a development dead end
