Add cache of query schemas to results view #3862
Conversation
alexet
force-pushed
the
alexet/bqrs-schema-cache
branch
from
c04893c to
605712b
Compare
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
extensions/ql-vscode/src/language-support/contextual/cached-operation.ts
Fixed
Show fixed
Hide fixed
extensions/ql-vscode/src/language-support/contextual/cached-operation.ts
Fixed
Show fixed
Hide fixed
alexet
force-pushed
the
alexet/bqrs-schema-cache
branch
from
605712b to
1982f52
Compare
|
I merged into main. The E2E tests should be fixed now. |
|
Hmmm....maybe not. |
|
I merged in main in another PR and got the same error: https://github.com/github/vscode-codeql/actions/runs/12282822206/job/34274881774?pr=3867 |
It is currently expected to fail due to a limitation in the E2E tests: they always install the released version, while the tests are testing the current version on the branch. See #3853. The E2E tests are not required, so they can be ignored. |
|
We need to do a new extension release in order to get the tests passing again. See #3869. |
aeisenberg
left a comment
aeisenberg
left a comment
There was a problem hiding this comment.
Looks good (after updating the changelog).
| if (this.generation !== origGeneration) { | ||
| // Cache was reset in the meantime so don't trust this | ||
| // result enough to cache it. | ||
| return result; | ||
| } |
Co-authored-by: Andrew Eisenberg <[email protected]>
The pagination this does is slow as it scan to compute the index of each page. By caching we mostly only do the scanning once and then later we can change page quickly. Unfortunately we must clear the cache on sorting though as that may reuse an existing bqrs.
For very large result sets (my test had 20,000,000 tuples) this takes the time to change page from about 10s to seemingly instant to me (I didn't precisely measure this, I just looked at a clock to get 10s) making looking through sorted data for certain results much easier.
I also removed a call to bqrs info with a page size of zero. Actually passing the pageSize of zero to the cli would result in an error, but as 0 is falsy so we treat it the same as the case without a pagesize. It seems clearer to just not provide the argument.
I am not sure why the E2E tests are failing though so I may need help debugging that.